[NET]: Fix fib_rules compatibility breakage
authorThomas Graf <tgraf@suug.ch>
Sat, 24 Mar 2007 19:46:02 +0000 (12:46 -0700)
committerDavid S. Miller <davem@sunset.davemloft.net>
Mon, 26 Mar 2007 01:48:00 +0000 (18:48 -0700)
Based upon a patch from Patrick McHardy.

The fib_rules netlink attribute policy introduced in 2.6.19 broke
userspace compatibilty. When specifying a rule with "from all"
or "to all", iproute adds a zero byte long netlink attribute,
but the policy requires all addresses to have a size equal to
sizeof(struct in_addr)/sizeof(struct in6_addr), resulting in a
validation error.

Check attribute length of FRA_SRC/FRA_DST in the generic framework
by letting the family specific rules implementation provide the
length of an address. Report an error if address length is non
zero but no address attribute is provided. Fix actual bug by
checking address length for non-zero instead of relying on
availability of attribute.

Signed-off-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/fib_rules.h
net/core/fib_rules.c
net/decnet/dn_rules.c
net/ipv4/fib_rules.c
net/ipv6/fib6_rules.c

index bc3c26494c3d2f4b578be0d12f4c20fd8e1b127c..d585ea9fa97d3887435e064922f5e65a62748851 100644 (file)
@@ -34,6 +34,7 @@ struct fib_rules_ops
        int                     family;
        struct list_head        list;
        int                     rule_size;
+       int                     addr_size;
 
        int                     (*action)(struct fib_rule *,
                                          struct flowi *, int,
index 3aea4e87d3d7c679abf051c4af2e6adbabbcac5b..d011819a805805480d14fd0d00832f2b794d9082 100644 (file)
@@ -152,6 +152,28 @@ out:
 
 EXPORT_SYMBOL_GPL(fib_rules_lookup);
 
+static int validate_rulemsg(struct fib_rule_hdr *frh, struct nlattr **tb,
+                           struct fib_rules_ops *ops)
+{
+       int err = -EINVAL;
+
+       if (frh->src_len)
+               if (tb[FRA_SRC] == NULL ||
+                   frh->src_len > (ops->addr_size * 8) ||
+                   nla_len(tb[FRA_SRC]) != ops->addr_size)
+                       goto errout;
+
+       if (frh->dst_len)
+               if (tb[FRA_DST] == NULL ||
+                   frh->dst_len > (ops->addr_size * 8) ||
+                   nla_len(tb[FRA_DST]) != ops->addr_size)
+                       goto errout;
+
+       err = 0;
+errout:
+       return err;
+}
+
 int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 {
        struct fib_rule_hdr *frh = nlmsg_data(nlh);
@@ -173,6 +195,10 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
        if (err < 0)
                goto errout;
 
+       err = validate_rulemsg(frh, tb, ops);
+       if (err < 0)
+               goto errout;
+
        rule = kzalloc(ops->rule_size, GFP_KERNEL);
        if (rule == NULL) {
                err = -ENOMEM;
@@ -260,6 +286,10 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
        if (err < 0)
                goto errout;
 
+       err = validate_rulemsg(frh, tb, ops);
+       if (err < 0)
+               goto errout;
+
        list_for_each_entry(rule, ops->rules_list, list) {
                if (frh->action && (frh->action != rule->action))
                        continue;
index b6c98ac93dc8737d96e54ac7f00dacacf90ae6f4..5e86dd5423024c3823cb1954e62b2d89c250441e 100644 (file)
@@ -109,8 +109,6 @@ errout:
 
 static struct nla_policy dn_fib_rule_policy[FRA_MAX+1] __read_mostly = {
        FRA_GENERIC_POLICY,
-       [FRA_SRC]       = { .type = NLA_U16 },
-       [FRA_DST]       = { .type = NLA_U16 },
 };
 
 static int dn_fib_rule_match(struct fib_rule *rule, struct flowi *fl, int flags)
@@ -133,7 +131,7 @@ static int dn_fib_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
        int err = -EINVAL;
        struct dn_fib_rule *r = (struct dn_fib_rule *)rule;
 
-       if (frh->src_len > 16 || frh->dst_len > 16 || frh->tos)
+       if (frh->tos)
                goto  errout;
 
        if (rule->table == RT_TABLE_UNSPEC) {
@@ -150,10 +148,10 @@ static int dn_fib_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
                }
        }
 
-       if (tb[FRA_SRC])
+       if (frh->src_len)
                r->src = nla_get_le16(tb[FRA_SRC]);
 
-       if (tb[FRA_DST])
+       if (frh->dst_len)
                r->dst = nla_get_le16(tb[FRA_DST]);
 
        r->src_len = frh->src_len;
@@ -176,10 +174,10 @@ static int dn_fib_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh,
        if (frh->dst_len && (r->dst_len != frh->dst_len))
                return 0;
 
-       if (tb[FRA_SRC] && (r->src != nla_get_le16(tb[FRA_SRC])))
+       if (frh->src_len && (r->src != nla_get_le16(tb[FRA_SRC])))
                return 0;
 
-       if (tb[FRA_DST] && (r->dst != nla_get_le16(tb[FRA_DST])))
+       if (frh->dst_len && (r->dst != nla_get_le16(tb[FRA_DST])))
                return 0;
 
        return 1;
@@ -249,6 +247,7 @@ int dn_fib_dump_rules(struct sk_buff *skb, struct netlink_callback *cb)
 static struct fib_rules_ops dn_fib_rules_ops = {
        .family         = AF_DECnet,
        .rule_size      = sizeof(struct dn_fib_rule),
+       .addr_size      = sizeof(u16),
        .action         = dn_fib_rule_action,
        .match          = dn_fib_rule_match,
        .configure      = dn_fib_rule_configure,
index b837c33e0404fc09a8e54ea7798d8e8b698657af..c660c074c76cc5360d0db1750e668ce0c4bd57a7 100644 (file)
@@ -171,8 +171,6 @@ static struct fib_table *fib_empty_table(void)
 
 static struct nla_policy fib4_rule_policy[FRA_MAX+1] __read_mostly = {
        FRA_GENERIC_POLICY,
-       [FRA_SRC]       = { .type = NLA_U32 },
-       [FRA_DST]       = { .type = NLA_U32 },
        [FRA_FLOW]      = { .type = NLA_U32 },
 };
 
@@ -183,8 +181,7 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
        int err = -EINVAL;
        struct fib4_rule *rule4 = (struct fib4_rule *) rule;
 
-       if (frh->src_len > 32 || frh->dst_len > 32 ||
-           (frh->tos & ~IPTOS_TOS_MASK))
+       if (frh->tos & ~IPTOS_TOS_MASK)
                goto errout;
 
        if (rule->table == RT_TABLE_UNSPEC) {
@@ -201,10 +198,10 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
                }
        }
 
-       if (tb[FRA_SRC])
+       if (frh->src_len)
                rule4->src = nla_get_be32(tb[FRA_SRC]);
 
-       if (tb[FRA_DST])
+       if (frh->dst_len)
                rule4->dst = nla_get_be32(tb[FRA_DST]);
 
 #ifdef CONFIG_NET_CLS_ROUTE
@@ -242,10 +239,10 @@ static int fib4_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh,
                return 0;
 #endif
 
-       if (tb[FRA_SRC] && (rule4->src != nla_get_be32(tb[FRA_SRC])))
+       if (frh->src_len && (rule4->src != nla_get_be32(tb[FRA_SRC])))
                return 0;
 
-       if (tb[FRA_DST] && (rule4->dst != nla_get_be32(tb[FRA_DST])))
+       if (frh->dst_len && (rule4->dst != nla_get_be32(tb[FRA_DST])))
                return 0;
 
        return 1;
@@ -309,6 +306,7 @@ static size_t fib4_rule_nlmsg_payload(struct fib_rule *rule)
 static struct fib_rules_ops fib4_rules_ops = {
        .family         = AF_INET,
        .rule_size      = sizeof(struct fib4_rule),
+       .addr_size      = sizeof(u32),
        .action         = fib4_rule_action,
        .match          = fib4_rule_match,
        .configure      = fib4_rule_configure,
index 0862809ffcf7abad0a4593126c9cccf3e8b29da5..ea3035b4e3e8d078e2b3c421aca48c2db0d51b7a 100644 (file)
@@ -131,8 +131,6 @@ static int fib6_rule_match(struct fib_rule *rule, struct flowi *fl, int flags)
 
 static struct nla_policy fib6_rule_policy[FRA_MAX+1] __read_mostly = {
        FRA_GENERIC_POLICY,
-       [FRA_SRC]       = { .len = sizeof(struct in6_addr) },
-       [FRA_DST]       = { .len = sizeof(struct in6_addr) },
 };
 
 static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
@@ -142,9 +140,6 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
        int err = -EINVAL;
        struct fib6_rule *rule6 = (struct fib6_rule *) rule;
 
-       if (frh->src_len > 128 || frh->dst_len > 128)
-               goto errout;
-
        if (rule->action == FR_ACT_TO_TBL) {
                if (rule->table == RT6_TABLE_UNSPEC)
                        goto errout;
@@ -155,11 +150,11 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
                }
        }
 
-       if (tb[FRA_SRC])
+       if (frh->src_len)
                nla_memcpy(&rule6->src.addr, tb[FRA_SRC],
                           sizeof(struct in6_addr));
 
-       if (tb[FRA_DST])
+       if (frh->dst_len)
                nla_memcpy(&rule6->dst.addr, tb[FRA_DST],
                           sizeof(struct in6_addr));
 
@@ -186,11 +181,11 @@ static int fib6_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh,
        if (frh->tos && (rule6->tclass != frh->tos))
                return 0;
 
-       if (tb[FRA_SRC] &&
+       if (frh->src_len &&
            nla_memcmp(tb[FRA_SRC], &rule6->src.addr, sizeof(struct in6_addr)))
                return 0;
 
-       if (tb[FRA_DST] &&
+       if (frh->dst_len &&
            nla_memcmp(tb[FRA_DST], &rule6->dst.addr, sizeof(struct in6_addr)))
                return 0;
 
@@ -240,6 +235,7 @@ static size_t fib6_rule_nlmsg_payload(struct fib_rule *rule)
 static struct fib_rules_ops fib6_rules_ops = {
        .family                 = AF_INET6,
        .rule_size              = sizeof(struct fib6_rule),
+       .addr_size              = sizeof(struct in6_addr),
        .action                 = fib6_rule_action,
        .match                  = fib6_rule_match,
        .configure              = fib6_rule_configure,