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 6bcb51c..3665948 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 368442e..05afe40 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 15cec9c..43cbfb0 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 9472c31..1f588fe 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;
 }