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@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

diff --git a/datapath/brc_procfs.c b/datapath/brc_procfs.c
index 6bcb51c2412bf44e348eabc1ab080512e58625fc..36659482a0232cbf6d608c3397c69d5ac4215055 100644 (file)
--- 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 368442e299b60c52fdd67a09c0ba6916761e8e05..05afe409e366d3a683b0bbe9f3276731be22125d 100644 (file)
--- 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 15cec9c912d467a807ab27c98d52dfdcb0bc9c9f..43cbfb0d9eebacd539fd4fee0efa4a9659016d0c 100644 (file)
--- 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 9472c316c3d23cee6290ca913673f707756d52ac..1f588fe88422337935f36edb3385b04d043f073c 100644 (file)
--- 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;
 }