thr3ads.net - LARTC - [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range [Mar 2007]

If this information is useful, please help other people find it:
Share via:

Luciano Ruete

2007-Mar-19 03:46 UTC

[BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

After an: 
# ip ru flush
I loose all my ip rules but the priority 0 one. 
root@sarasvati:~# ip ru
0:      from all lookup 255
root@sarasvati:~#  

Ok with that, but now i''m not able to insert any new rule.
This leads to a total loose of conectivity.

root@sarasvati:~# ip ru add from all table default
RTNETLINK answers: Numerical result out of range
root@sarasvati:~# ip ru add from all lookup main
RTNETLINK answers: Numerical result out of range

Even seting the priority value by hand, i got the same error:

root@sarasvati:~# ip ru add from all lookup main priority 32766
RTNETLINK answers: Numerical result out of range

To be able to send this e-mail without rebooting i had to insert my gw ip 
routes in table 255.

Is this a bug in iproute?

Some adiotional data:
# ip -V
ip utility, iproute2-ss060323
# uname -a
Linux sarasvati 2.6.20-5-386 #2 Sat Jan 6 14:44:57 UTC 2007 i686 GNU/Linux
# cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=7.04
DISTRIB_CODENAME=feisty
DISTRIB_DESCRIPTION="Ubuntu feisty (development branch)"
-- 
Luciano

Patrick McHardy

2007-Mar-19 05:54 UTC

head link

Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Luciano Ruete wrote:> After an: 
> # ip ru flush
> I loose all my ip rules but the priority 0 one. 
> root@sarasvati:~# ip ru
> 0:      from all lookup 255
> root@sarasvati:~#  
> 
> Ok with that, but now i''m not able to insert any new rule.
> This leads to a total loose of conectivity.
> 
> root@sarasvati:~# ip ru add from all table default
> RTNETLINK answers: Numerical result out of range
> root@sarasvati:~# ip ru add from all lookup main
> RTNETLINK answers: Numerical result out of range
> 
> Even seting the priority value by hand, i got the same error:
> 
> root@sarasvati:~# ip ru add from all lookup main priority 32766
> RTNETLINK answers: Numerical result out of range
> 
> To be able to send this e-mail without rebooting i had to insert my gw ip 
> routes in table 255.
> 
> Is this a bug in iproute?
> 
> Some adiotional data:
> ip utility, iproute2-ss060323
> Linux sarasvati 2.6.20-5-386 #2 Sat Jan 6 14:44:57 UTC 2007 i686 GNU/Linux

The problem seems to be the nla policy added in 2.6.19 or 2.6.20.
When specifying a prefix as "all", iproute adds a zero byte long
attribute (FRA_SRC in this case). The IPv4 fib_rules policy states
that it has to be exactly 4 bytes long, which makes validation fail.
This also affects IPv6 and DECnet.

I would argue that iproute is broken and shouldn''t add a zero
byte long attribute, but we still need to make sure the kernel
accepts these attributes as valid.

Thomas, I can''t see a clean way to fix this right now that
doesn''t either bloat struct nla_policy or removes FRA_SRC/FRA_DST
from the policy, could you please look into this? Thanks.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thomas Graf

2007-Mar-19 15:25 UTC

head link

Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

* Patrick McHardy <kaber@trash.net> 2007-03-19
06:54> Thomas, I can''t see a clean way to fix this right now that
> doesn''t either bloat struct nla_policy or removes FRA_SRC/FRA_DST
> from the policy, could you please look into this? Thanks.
I guess the only way is to remove FRA_SRC/FRA_DST from the policy
and validate it in configure() based on src_len/dst_len.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patrick McHardy

2007-Mar-20 06:19 UTC

head link

Re: [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Thomas Graf wrote:> * Patrick McHardy <kaber@trash.net> 2007-03-19 06:54
> 
>>Thomas, I can''t see a clean way to fix this right now that
>>doesn''t either bloat struct nla_policy or removes
FRA_SRC/FRA_DST
>>from the policy, could you please look into this? Thanks.
> 
> 
> I guess the only way is to remove FRA_SRC/FRA_DST from the policy
> and validate it in configure() based on src_len/dst_len.

Its not too pretty, but I agree. This patch fixes the problem.
I''ll also push it to -stable.


_______________________________________________
LARTC mailing list
LARTC@mailman.ds9a.nl
http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc

Patrick McHardy

2007-Mar-20 06:42 UTC

head link

Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Patrick McHardy wrote:> [NET]: Fix fib_rules compatibility breakage
I forgot to remove FRA_SRC/FRA_DST from fib6_rule_policy.
Updated patch attached.

Thomas Graf

2007-Mar-20 16:40 UTC

head link

[NET]: Fix fib_rules compatibility breakage

Based on Patrick''s patch:
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>

Index: net-2.6/include/net/fib_rules.h
==================================================================---
net-2.6.orig/include/net/fib_rules.h	2007-03-20 15:38:19.000000000 +0100
+++ net-2.6/include/net/fib_rules.h	2007-03-20 16:01:31.000000000 +0100
@@ -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: net-2.6/net/core/fib_rules.c
==================================================================---
net-2.6.orig/net/core/fib_rules.c	2007-03-20 15:37:39.000000000 +0100
+++ net-2.6/net/core/fib_rules.c	2007-03-20 15:56:59.000000000 +0100
@@ -173,6 +173,19 @@ int fib_nl_newrule(struct sk_buff *skb, 
 	if (err < 0)
 		goto errout;
 
+	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;
+
 	rule = kzalloc(ops->rule_size, GFP_KERNEL);
 	if (rule == NULL) {
 		err = -ENOMEM;
Index: net-2.6/net/decnet/dn_rules.c
==================================================================---
net-2.6.orig/net/decnet/dn_rules.c	2007-03-20 15:35:26.000000000 +0100
+++ net-2.6/net/decnet/dn_rules.c	2007-03-20 15:58:29.000000000 +0100
@@ -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 
 	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 
 		}
 	}
 
-	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 fi
 	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 *sk
 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: net-2.6/net/ipv4/fib_rules.c
==================================================================---
net-2.6.orig/net/ipv4/fib_rules.c	2007-03-20 15:46:16.000000000 +0100
+++ net-2.6/net/ipv4/fib_rules.c	2007-03-20 15:55:08.000000000 +0100
@@ -171,8 +171,6 @@ static struct fib_table *fib_empty_table
 
 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 fi
 	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 fi
 		}
 	}
 
-	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_
 		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(st
 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: net-2.6/net/ipv6/fib6_rules.c
==================================================================---
net-2.6.orig/net/ipv6/fib6_rules.c	2007-03-20 15:48:50.000000000 +0100
+++ net-2.6/net/ipv6/fib6_rules.c	2007-03-20 15:57:44.000000000 +0100
@@ -131,8 +131,6 @@ static int fib6_rule_match(struct fib_ru
 
 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 fi
 	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 fi
 		}
 	}
 
-	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_
 	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(st
 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,
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patrick McHardy

2007-Mar-20 16:59 UTC

head link

Re: [NET]: Fix fib_rules compatibility breakage

Thomas Graf wrote:> @@ -242,10 +239,10 @@ static int fib4_rule_compare(struct fib_
>  		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;
>  

The presence of the attributes when src_len/dst_len is non-zero
is only verified in fib_newrule, so this looks like it might crash
when something broken sets src_len/dst_len to a non-zero value
without actually adding the attributes.

Other than that it looks fine.

Thomas Graf

2007-Mar-20 18:15 UTC

head link

[NET]: Fix fib_rules compatibility breakage

* Patrick McHardy <kaber@trash.net> 2007-03-20
17:59> The presence of the attributes when src_len/dst_len is non-zero
> is only verified in fib_newrule, so this looks like it might crash
> when something broken sets src_len/dst_len to a non-zero value
> without actually adding the attributes.
You''re right, we need to validate in fib_nl_delrule() as well.

Based on Patrick''s patch:
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>

Index: net-2.6/include/net/fib_rules.h
==================================================================---
net-2.6.orig/include/net/fib_rules.h	2007-03-20 16:49:06.000000000 +0100
+++ net-2.6/include/net/fib_rules.h	2007-03-20 17:22:35.000000000 +0100
@@ -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: net-2.6/net/core/fib_rules.c
==================================================================---
net-2.6.orig/net/core/fib_rules.c	2007-03-20 16:49:06.000000000 +0100
+++ net-2.6/net/core/fib_rules.c	2007-03-20 19:09:52.000000000 +0100
@@ -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, 
 	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, 
 	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: net-2.6/net/decnet/dn_rules.c
==================================================================---
net-2.6.orig/net/decnet/dn_rules.c	2007-03-20 16:49:06.000000000 +0100
+++ net-2.6/net/decnet/dn_rules.c	2007-03-20 17:22:35.000000000 +0100
@@ -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 
 	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 
 		}
 	}
 
-	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 fi
 	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 *sk
 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: net-2.6/net/ipv4/fib_rules.c
==================================================================---
net-2.6.orig/net/ipv4/fib_rules.c	2007-03-20 16:49:06.000000000 +0100
+++ net-2.6/net/ipv4/fib_rules.c	2007-03-20 17:22:35.000000000 +0100
@@ -171,8 +171,6 @@ static struct fib_table *fib_empty_table
 
 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 fi
 	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 fi
 		}
 	}
 
-	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_
 		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(st
 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: net-2.6/net/ipv6/fib6_rules.c
==================================================================---
net-2.6.orig/net/ipv6/fib6_rules.c	2007-03-20 16:49:06.000000000 +0100
+++ net-2.6/net/ipv6/fib6_rules.c	2007-03-20 17:22:35.000000000 +0100
@@ -131,8 +131,6 @@ static int fib6_rule_match(struct fib_ru
 
 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 fi
 	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 fi
 		}
 	}
 
-	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_
 	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(st
 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,
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patrick McHardy

2007-Mar-20 19:58 UTC

head link

Re: [NET]: Fix fib_rules compatibility breakage

Thomas Graf wrote:> * Patrick McHardy <kaber@trash.net> 2007-03-20 17:59
> 
>>The presence of the attributes when src_len/dst_len is non-zero
>>is only verified in fib_newrule, so this looks like it might crash
>>when something broken sets src_len/dst_len to a non-zero value
>>without actually adding the attributes.
> 
> 
> You''re right, we need to validate in fib_nl_delrule() as well.
> 
> Based on Patrick''s patch:
> 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>
This looks good, thanks.

Signed-off-by: Patrick McHardy <kaber@trash.net>

David Miller

2007-Mar-24 19:48 UTC

head link

Re: [NET]: Fix fib_rules compatibility breakage

From: Patrick McHardy <kaber@trash.net>
Date: Tue, 20 Mar 2007 20:58:55 +0100
> Thomas Graf wrote:
> > * Patrick McHardy <kaber@trash.net> 2007-03-20 17:59
> > 
> >>The presence of the attributes when src_len/dst_len is non-zero
> >>is only verified in fib_newrule, so this looks like it might crash
> >>when something broken sets src_len/dst_len to a non-zero value
> >>without actually adding the attributes.
> > 
> > 
> > You''re right, we need to validate in fib_nl_delrule() as
well.
> > 
> > Based on Patrick''s patch:
> > 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>
> 
> This looks good, thanks.
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>
Applied, thanks guys, I''ll push this to 2.6.20-stable as well.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reasonably Related Threads

Search for more apparently analagous threads

LARTC - Mar 2007 - [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

[BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Re: [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range

[NET]: Fix fib_rules compatibility breakage

Re: [NET]: Fix fib_rules compatibility breakage

[NET]: Fix fib_rules compatibility breakage

Re: [NET]: Fix fib_rules compatibility breakage

Re: [NET]: Fix fib_rules compatibility breakage

Reasonably Related Threads