[ovs-dev] [netlink v4+ 2/3] datapath: Make VERIFY_NUL_STRING verify the string length too.
Ben Pfaff
blp at nicira.com
Thu Jan 20 00:22:02 UTC 2011
On Wed, Jan 19, 2011 at 12:19:28PM -0800, Jesse Gross wrote:
> On Wed, Jan 12, 2011 at 1:23 PM, Ben Pfaff <blp at nicira.com> wrote:
> > It turns out that just defining NLA_NUL_STRING to an innocuous value is
> > not sufficient: Linux before 2.6.19 doesn't define a 'len' member in
> > struct nla_policy at all (it was named 'minlen' and had different
> > semantics), so attempting to initialize it caused compile errors.
> >
> > It's better to use HAVE_NLA_NUL_STRING than a version check because the
> > Xen 2.6.18 kernels backport NLA_NUL_STRING and the nla_policy changes.
> >
> > Grouping things this way also makes it clearer what needs to be deleted
> > when upstreaming.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
>
> Acked-by: Jesse Gross <jesse at nicira.com>
Thanks, I've now stuck appropriate bits of this into other commits and
moved the remainder just after "datapath: Make VERIFY_NUL_STRING return
a negative error code." I'm appending this revised version just for
completeness.
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <blp at nicira.com>
Date: Wed, 19 Jan 2011 16:16:08 -0800
Subject: [PATCH] datapath: Make VERIFY_NUL_STRING verify the string length too.
It's better to use HAVE_NLA_NUL_STRING than a version check because the
Xen 2.6.18 kernels backport NLA_NUL_STRING and the nla_policy changes.
Just defining NLA_NUL_STRING to an innocuous value doesn't work, because
Linux before 2.6.19 doesn't define a 'len' member in struct nla_policy at
all (it was named 'minlen' and had different semantics), so attempting to
initialize it caused compile errors.
Grouping things this way also makes it clearer what needs to be deleted
when upstreaming.
Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Jesse Gross <jesse at nicira.com>
---
datapath/brc_procfs.c | 16 ++-----------
datapath/brc_procfs.h | 5 +++-
datapath/brcompat.c | 8 +++++-
.../linux-2.6/compat-2.6/include/net/netlink.h | 23 +++++++++++++------
4 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/datapath/brc_procfs.c b/datapath/brc_procfs.c
index 6bcb51c..3665948 100644
--- a/datapath/brc_procfs.c
+++ b/datapath/brc_procfs.c
@@ -81,13 +81,6 @@ static struct proc_dir_entry *brc_open_dir(const char *dir_name,
return *dirp;
}
-/* Maximum length of the BRC_GENL_A_PROC_DIR and BRC_GENL_A_PROC_NAME strings.
- * If we could depend on supporting NLA_NUL_STRING and the .len member in
- * Generic Netlink policy, then we could just put this in brc_genl_policy (and
- * simplify brc_genl_set_proc() below too), but upstream 2.6.18 does not have
- * either. */
-#define BRC_NAME_LEN_MAX 32
-
int brc_genl_set_proc(struct sk_buff *skb, struct genl_info *info)
{
struct proc_dir_entry *dir, *entry;
@@ -95,18 +88,15 @@ int brc_genl_set_proc(struct sk_buff *skb, struct genl_info *info)
char *data;
if (!info->attrs[BRC_GENL_A_PROC_DIR] ||
- VERIFY_NUL_STRING(info->attrs[BRC_GENL_A_PROC_DIR]) ||
+ VERIFY_NUL_STRING(info->attrs[BRC_GENL_A_PROC_DIR], BRC_NAME_LEN_MAX) ||
!info->attrs[BRC_GENL_A_PROC_NAME] ||
- VERIFY_NUL_STRING(info->attrs[BRC_GENL_A_PROC_NAME]) ||
+ VERIFY_NUL_STRING(info->attrs[BRC_GENL_A_PROC_NAME], BRC_NAME_LEN_MAX) ||
(info->attrs[BRC_GENL_A_PROC_DATA] &&
- VERIFY_NUL_STRING(info->attrs[BRC_GENL_A_PROC_DATA])))
+ VERIFY_NUL_STRING(info->attrs[BRC_GENL_A_PROC_DATA], INT_MAX)))
return -EINVAL;
dir_name = nla_data(info->attrs[BRC_GENL_A_PROC_DIR]);
name = nla_data(info->attrs[BRC_GENL_A_PROC_NAME]);
- if (strlen(dir_name) > BRC_NAME_LEN_MAX ||
- strlen(name) > BRC_NAME_LEN_MAX)
- return -EINVAL;
if (!strcmp(dir_name, "net/vlan"))
dir = brc_open_dir("vlan", proc_net, &proc_vlan_dir);
diff --git a/datapath/brc_procfs.h b/datapath/brc_procfs.h
index 368442e..05afe40 100644
--- a/datapath/brc_procfs.h
+++ b/datapath/brc_procfs.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009 Nicira Networks.
+ * Copyright (c) 2009, 2011 Nicira Networks.
* Distributed under the terms of the GNU GPL version 2.
*
* Significant portions of this file may be copied from parts of the Linux
@@ -12,6 +12,9 @@
struct sk_buff;
struct genl_info;
+/* Maximum length of BRC_GENL_A_PROC_DIR and BRC_GENL_A_PROC_NAME strings. */
+#define BRC_NAME_LEN_MAX 32
+
void brc_procfs_exit(void);
int brc_genl_set_proc(struct sk_buff *skb, struct genl_info *info);
diff --git a/datapath/brcompat.c b/datapath/brcompat.c
index 15cec9c..43cbfb0 100644
--- a/datapath/brcompat.c
+++ b/datapath/brcompat.c
@@ -408,9 +408,13 @@ static struct genl_ops brc_genl_ops_query_dp = {
static struct nla_policy brc_genl_policy[BRC_GENL_A_MAX + 1] = {
[BRC_GENL_A_ERR_CODE] = { .type = NLA_U32 },
- [BRC_GENL_A_PROC_DIR] = { .type = NLA_NUL_STRING },
- [BRC_GENL_A_PROC_NAME] = { .type = NLA_NUL_STRING },
+#ifdef HAVE_NLA_NUL_STRING
+ [BRC_GENL_A_PROC_DIR] = { .type = NLA_NUL_STRING,
+ .len = BRC_NAME_LEN_MAX },
+ [BRC_GENL_A_PROC_NAME] = { .type = NLA_NUL_STRING,
+ .len = BRC_NAME_LEN_MAX },
[BRC_GENL_A_PROC_DATA] = { .type = NLA_NUL_STRING },
+#endif
[BRC_GENL_A_FDB_DATA] = { .type = NLA_UNSPEC },
};
diff --git a/datapath/linux-2.6/compat-2.6/include/net/netlink.h b/datapath/linux-2.6/compat-2.6/include/net/netlink.h
index 9472c31..1f588fe 100644
--- a/datapath/linux-2.6/compat-2.6/include/net/netlink.h
+++ b/datapath/linux-2.6/compat-2.6/include/net/netlink.h
@@ -5,16 +5,25 @@
#include_next <net/netlink.h>
#ifndef HAVE_NLA_NUL_STRING
-#define NLA_NUL_STRING NLA_STRING
-
-static inline int VERIFY_NUL_STRING(struct nlattr *attr)
+static inline int VERIFY_NUL_STRING(struct nlattr *attr, int maxlen)
{
- return (!attr || (nla_len(attr)
- && memchr(nla_data(attr), '\0', nla_len(attr)))
- ? 0 : -EINVAL);
+ char *s;
+ int len;
+ if (!attr)
+ return 0;
+
+ len = nla_len(attr);
+ if (len >= maxlen)
+ return -EINVAL;
+
+ s = nla_data(attr);
+ if (s[len - 1] != '\0')
+ return -EINVAL;
+
+ return 0;
}
#else
-static inline int VERIFY_NUL_STRING(struct nlattr *attr)
+static inline int VERIFY_NUL_STRING(struct nlattr *attr, int maxlen)
{
return 0;
}
--
1.7.1
More information about the dev
mailing list