* [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result @ 2007-03-19 3:46 Luciano Ruete 2007-03-19 5:54 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy 0 siblings, 1 reply; 18+ messages in thread From: Luciano Ruete @ 2007-03-19 3:46 UTC (permalink / raw) To: lartc 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þisty DISTRIB_DESCRIPTION="Ubuntu feisty (development branch)" -- Luciano _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result 2007-03-19 3:46 [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result Luciano Ruete @ 2007-03-19 5:54 ` Patrick McHardy 0 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-19 5:54 UTC (permalink / raw) To: Luciano Ruete; +Cc: lartc, Linux Netdev List, Thomas Graf 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. _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range @ 2007-03-19 5:54 ` Patrick McHardy 0 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-19 5:54 UTC (permalink / raw) To: Luciano Ruete; +Cc: lartc, Linux Netdev List, Thomas Graf 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result 2007-03-19 5:54 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy @ 2007-03-19 15:25 ` Thomas Graf -1 siblings, 0 replies; 18+ messages in thread From: Thomas Graf @ 2007-03-19 15:25 UTC (permalink / raw) To: Patrick McHardy; +Cc: Luciano Ruete, lartc, Linux Netdev List * 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. _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range @ 2007-03-19 15:25 ` Thomas Graf 0 siblings, 0 replies; 18+ messages in thread From: Thomas Graf @ 2007-03-19 15:25 UTC (permalink / raw) To: Patrick McHardy; +Cc: Luciano Ruete, lartc, Linux Netdev List * 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result 2007-03-19 15:25 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Thomas Graf @ 2007-03-20 6:19 ` Patrick McHardy -1 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 6:19 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc [-- Attachment #1: Type: text/plain, Size: 494 bytes --] 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. [-- Attachment #2: x --] [-- Type: text/plain, Size: 3942 bytes --] [NET]: Fix fib_rules compatibility breakage 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. Fix by only looking at the FRA_SRC/FRA_DST attributes if src_len or dst_len is larger than zero. DECnet is unaffected since iproute doesn't support specifying addresses as "all". Signed-off-by: Patrick McHardy <kaber@trash.net> --- commit 39f42dd26f1f9c93b9700e1bace540ed9bb94e46 tree ecc71ef742d9d636bf129b34ae7a18173377ccc0 parent db98e0b434a6265c451ffe94ec0a29b8d0aaf587 author Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:08:38 +0100 committer Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:08:38 +0100 net/ipv4/fib_rules.c | 18 ++++++++++++------ net/ipv6/fib6_rules.c | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index b837c33..9524b2e 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -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 }, }; @@ -187,6 +185,12 @@ static int fib4_rule_configure(struct fi (frh->tos & ~IPTOS_TOS_MASK)) goto errout; + if (frh->src_len && tb[FRA_SRC] && nla_len(tb[FRA_SRC]) != sizeof(u32)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && nla_len(tb[FRA_DST]) != sizeof(u32)) + goto errout; + if (rule->table == RT_TABLE_UNSPEC) { if (rule->action == FR_ACT_TO_TBL) { struct fib_table *table; @@ -201,10 +205,10 @@ static int fib4_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) rule4->src = nla_get_be32(tb[FRA_SRC]); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) rule4->dst = nla_get_be32(tb[FRA_DST]); #ifdef CONFIG_NET_CLS_ROUTE @@ -242,10 +246,12 @@ #ifdef CONFIG_NET_CLS_ROUTE return 0; #endif - if (tb[FRA_SRC] && (rule4->src != nla_get_be32(tb[FRA_SRC]))) + if (frh->src_len && tb[FRA_SRC] && + (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 && tb[FRA_DST] && + (rule4->dst != nla_get_be32(tb[FRA_DST]))) return 0; return 1; diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 0862809..a15244e 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -145,6 +145,14 @@ static int fib6_rule_configure(struct fi if (frh->src_len > 128 || frh->dst_len > 128) goto errout; + if (frh->src_len && tb[FRA_SRC] && + nla_len(tb[FRA_SRC]) != sizeof(struct in6_addr)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && + nla_len(tb[FRA_DST]) != sizeof(struct in6_addr)) + goto errout; + if (rule->action == FR_ACT_TO_TBL) { if (rule->table == RT6_TABLE_UNSPEC) goto errout; @@ -155,11 +163,11 @@ static int fib6_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) nla_memcpy(&rule6->src.addr, tb[FRA_SRC], sizeof(struct in6_addr)); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) nla_memcpy(&rule6->dst.addr, tb[FRA_DST], sizeof(struct in6_addr)); @@ -186,11 +194,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 && tb[FRA_SRC] && nla_memcmp(tb[FRA_SRC], &rule6->src.addr, sizeof(struct in6_addr))) return 0; - if (tb[FRA_DST] && + if (frh->dst_len && tb[FRA_DST] && nla_memcmp(tb[FRA_DST], &rule6->dst.addr, sizeof(struct in6_addr))) return 0; [-- Attachment #3: Type: text/plain, Size: 143 bytes --] _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range @ 2007-03-20 6:19 ` Patrick McHardy 0 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 6:19 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc [-- Attachment #1: Type: text/plain, Size: 494 bytes --] 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. [-- Attachment #2: x --] [-- Type: text/plain, Size: 3942 bytes --] [NET]: Fix fib_rules compatibility breakage 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. Fix by only looking at the FRA_SRC/FRA_DST attributes if src_len or dst_len is larger than zero. DECnet is unaffected since iproute doesn't support specifying addresses as "all". Signed-off-by: Patrick McHardy <kaber@trash.net> --- commit 39f42dd26f1f9c93b9700e1bace540ed9bb94e46 tree ecc71ef742d9d636bf129b34ae7a18173377ccc0 parent db98e0b434a6265c451ffe94ec0a29b8d0aaf587 author Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:08:38 +0100 committer Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:08:38 +0100 net/ipv4/fib_rules.c | 18 ++++++++++++------ net/ipv6/fib6_rules.c | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index b837c33..9524b2e 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -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 }, }; @@ -187,6 +185,12 @@ static int fib4_rule_configure(struct fi (frh->tos & ~IPTOS_TOS_MASK)) goto errout; + if (frh->src_len && tb[FRA_SRC] && nla_len(tb[FRA_SRC]) != sizeof(u32)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && nla_len(tb[FRA_DST]) != sizeof(u32)) + goto errout; + if (rule->table == RT_TABLE_UNSPEC) { if (rule->action == FR_ACT_TO_TBL) { struct fib_table *table; @@ -201,10 +205,10 @@ static int fib4_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) rule4->src = nla_get_be32(tb[FRA_SRC]); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) rule4->dst = nla_get_be32(tb[FRA_DST]); #ifdef CONFIG_NET_CLS_ROUTE @@ -242,10 +246,12 @@ #ifdef CONFIG_NET_CLS_ROUTE return 0; #endif - if (tb[FRA_SRC] && (rule4->src != nla_get_be32(tb[FRA_SRC]))) + if (frh->src_len && tb[FRA_SRC] && + (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 && tb[FRA_DST] && + (rule4->dst != nla_get_be32(tb[FRA_DST]))) return 0; return 1; diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 0862809..a15244e 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -145,6 +145,14 @@ static int fib6_rule_configure(struct fi if (frh->src_len > 128 || frh->dst_len > 128) goto errout; + if (frh->src_len && tb[FRA_SRC] && + nla_len(tb[FRA_SRC]) != sizeof(struct in6_addr)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && + nla_len(tb[FRA_DST]) != sizeof(struct in6_addr)) + goto errout; + if (rule->action == FR_ACT_TO_TBL) { if (rule->table == RT6_TABLE_UNSPEC) goto errout; @@ -155,11 +163,11 @@ static int fib6_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) nla_memcpy(&rule6->src.addr, tb[FRA_SRC], sizeof(struct in6_addr)); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) nla_memcpy(&rule6->dst.addr, tb[FRA_DST], sizeof(struct in6_addr)); @@ -186,11 +194,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 && tb[FRA_SRC] && nla_memcmp(tb[FRA_SRC], &rule6->src.addr, sizeof(struct in6_addr))) return 0; - if (tb[FRA_DST] && + if (frh->dst_len && tb[FRA_DST] && nla_memcmp(tb[FRA_DST], &rule6->dst.addr, sizeof(struct in6_addr))) return 0; [-- Attachment #3: Type: text/plain, Size: 143 bytes --] _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result 2007-03-20 6:19 ` [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy @ 2007-03-20 6:42 ` Patrick McHardy -1 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 6:42 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc [-- Attachment #1: Type: text/plain, Size: 153 bytes --] Patrick McHardy wrote: > [NET]: Fix fib_rules compatibility breakage I forgot to remove FRA_SRC/FRA_DST from fib6_rule_policy. Updated patch attached. [-- Attachment #2: x --] [-- Type: text/plain, Size: 4282 bytes --] [NET]: Fix fib_rules compatibility breakage 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. Fix by only looking at the FRA_SRC/FRA_DST attributes if src_len or dst_len is larger than zero. DECnet is unaffected since iproute doesn't support specifying addresses as "all". Signed-off-by: Patrick McHardy <kaber@trash.net> --- commit 676307508e675dcf434f4f18e619b195f2f503ad tree aa87342144b3b19ebddb5011380c3ac96c38dbbc parent db98e0b434a6265c451ffe94ec0a29b8d0aaf587 author Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:08:38 +0100 committer Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:42:19 +0100 net/ipv4/fib_rules.c | 18 ++++++++++++------ net/ipv6/fib6_rules.c | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index b837c33..9524b2e 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -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 }, }; @@ -187,6 +185,12 @@ static int fib4_rule_configure(struct fi (frh->tos & ~IPTOS_TOS_MASK)) goto errout; + if (frh->src_len && tb[FRA_SRC] && nla_len(tb[FRA_SRC]) != sizeof(u32)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && nla_len(tb[FRA_DST]) != sizeof(u32)) + goto errout; + if (rule->table == RT_TABLE_UNSPEC) { if (rule->action == FR_ACT_TO_TBL) { struct fib_table *table; @@ -201,10 +205,10 @@ static int fib4_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) rule4->src = nla_get_be32(tb[FRA_SRC]); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) rule4->dst = nla_get_be32(tb[FRA_DST]); #ifdef CONFIG_NET_CLS_ROUTE @@ -242,10 +246,12 @@ #ifdef CONFIG_NET_CLS_ROUTE return 0; #endif - if (tb[FRA_SRC] && (rule4->src != nla_get_be32(tb[FRA_SRC]))) + if (frh->src_len && tb[FRA_SRC] && + (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 && tb[FRA_DST] && + (rule4->dst != nla_get_be32(tb[FRA_DST]))) return 0; return 1; diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 0862809..6331306 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -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, @@ -145,6 +143,14 @@ static int fib6_rule_configure(struct fi if (frh->src_len > 128 || frh->dst_len > 128) goto errout; + if (frh->src_len && tb[FRA_SRC] && + nla_len(tb[FRA_SRC]) != sizeof(struct in6_addr)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && + nla_len(tb[FRA_DST]) != sizeof(struct in6_addr)) + goto errout; + if (rule->action == FR_ACT_TO_TBL) { if (rule->table == RT6_TABLE_UNSPEC) goto errout; @@ -155,11 +161,11 @@ static int fib6_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) nla_memcpy(&rule6->src.addr, tb[FRA_SRC], sizeof(struct in6_addr)); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) nla_memcpy(&rule6->dst.addr, tb[FRA_DST], sizeof(struct in6_addr)); @@ -186,11 +192,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 && tb[FRA_SRC] && nla_memcmp(tb[FRA_SRC], &rule6->src.addr, sizeof(struct in6_addr))) return 0; - if (tb[FRA_DST] && + if (frh->dst_len && tb[FRA_DST] && nla_memcmp(tb[FRA_DST], &rule6->dst.addr, sizeof(struct in6_addr))) return 0; [-- Attachment #3: Type: text/plain, Size: 143 bytes --] _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range @ 2007-03-20 6:42 ` Patrick McHardy 0 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 6:42 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc [-- Attachment #1: Type: text/plain, Size: 153 bytes --] Patrick McHardy wrote: > [NET]: Fix fib_rules compatibility breakage I forgot to remove FRA_SRC/FRA_DST from fib6_rule_policy. Updated patch attached. [-- Attachment #2: x --] [-- Type: text/plain, Size: 4282 bytes --] [NET]: Fix fib_rules compatibility breakage 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. Fix by only looking at the FRA_SRC/FRA_DST attributes if src_len or dst_len is larger than zero. DECnet is unaffected since iproute doesn't support specifying addresses as "all". Signed-off-by: Patrick McHardy <kaber@trash.net> --- commit 676307508e675dcf434f4f18e619b195f2f503ad tree aa87342144b3b19ebddb5011380c3ac96c38dbbc parent db98e0b434a6265c451ffe94ec0a29b8d0aaf587 author Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:08:38 +0100 committer Patrick McHardy <kaber@trash.net> Tue, 20 Mar 2007 07:42:19 +0100 net/ipv4/fib_rules.c | 18 ++++++++++++------ net/ipv6/fib6_rules.c | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index b837c33..9524b2e 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -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 }, }; @@ -187,6 +185,12 @@ static int fib4_rule_configure(struct fi (frh->tos & ~IPTOS_TOS_MASK)) goto errout; + if (frh->src_len && tb[FRA_SRC] && nla_len(tb[FRA_SRC]) != sizeof(u32)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && nla_len(tb[FRA_DST]) != sizeof(u32)) + goto errout; + if (rule->table == RT_TABLE_UNSPEC) { if (rule->action == FR_ACT_TO_TBL) { struct fib_table *table; @@ -201,10 +205,10 @@ static int fib4_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) rule4->src = nla_get_be32(tb[FRA_SRC]); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) rule4->dst = nla_get_be32(tb[FRA_DST]); #ifdef CONFIG_NET_CLS_ROUTE @@ -242,10 +246,12 @@ #ifdef CONFIG_NET_CLS_ROUTE return 0; #endif - if (tb[FRA_SRC] && (rule4->src != nla_get_be32(tb[FRA_SRC]))) + if (frh->src_len && tb[FRA_SRC] && + (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 && tb[FRA_DST] && + (rule4->dst != nla_get_be32(tb[FRA_DST]))) return 0; return 1; diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 0862809..6331306 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -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, @@ -145,6 +143,14 @@ static int fib6_rule_configure(struct fi if (frh->src_len > 128 || frh->dst_len > 128) goto errout; + if (frh->src_len && tb[FRA_SRC] && + nla_len(tb[FRA_SRC]) != sizeof(struct in6_addr)) + goto errout; + + if (frh->dst_len && tb[FRA_DST] && + nla_len(tb[FRA_DST]) != sizeof(struct in6_addr)) + goto errout; + if (rule->action == FR_ACT_TO_TBL) { if (rule->table == RT6_TABLE_UNSPEC) goto errout; @@ -155,11 +161,11 @@ static int fib6_rule_configure(struct fi } } - if (tb[FRA_SRC]) + if (frh->src_len && tb[FRA_SRC]) nla_memcpy(&rule6->src.addr, tb[FRA_SRC], sizeof(struct in6_addr)); - if (tb[FRA_DST]) + if (frh->dst_len && tb[FRA_DST]) nla_memcpy(&rule6->dst.addr, tb[FRA_DST], sizeof(struct in6_addr)); @@ -186,11 +192,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 && tb[FRA_SRC] && nla_memcmp(tb[FRA_SRC], &rule6->src.addr, sizeof(struct in6_addr))) return 0; - if (tb[FRA_DST] && + if (frh->dst_len && tb[FRA_DST] && nla_memcmp(tb[FRA_DST], &rule6->dst.addr, sizeof(struct in6_addr))) return 0; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [LARTC] [NET]: Fix fib_rules compatibility breakage 2007-03-20 6:42 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy @ 2007-03-20 16:40 ` Thomas Graf -1 siblings, 0 replies; 18+ messages in thread From: Thomas Graf @ 2007-03-20 16:40 UTC (permalink / raw) To: Patrick McHardy; +Cc: Linux Netdev List, David S. Miller, lartc 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, _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* [NET]: Fix fib_rules compatibility breakage @ 2007-03-20 16:40 ` Thomas Graf 0 siblings, 0 replies; 18+ messages in thread From: Thomas Graf @ 2007-03-20 16:40 UTC (permalink / raw) To: Patrick McHardy; +Cc: Linux Netdev List, David S. Miller, lartc 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, ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LARTC] Re: [NET]: Fix fib_rules compatibility breakage 2007-03-20 16:40 ` Thomas Graf @ 2007-03-20 16:59 ` Patrick McHardy -1 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 16:59 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc 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. _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [NET]: Fix fib_rules compatibility breakage @ 2007-03-20 16:59 ` Patrick McHardy 0 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 16:59 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LARTC] [NET]: Fix fib_rules compatibility breakage 2007-03-20 16:59 ` Patrick McHardy @ 2007-03-20 18:15 ` Thomas Graf -1 siblings, 0 replies; 18+ messages in thread From: Thomas Graf @ 2007-03-20 18:15 UTC (permalink / raw) To: Patrick McHardy; +Cc: Linux Netdev List, David S. Miller, lartc * 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, _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* [NET]: Fix fib_rules compatibility breakage @ 2007-03-20 18:15 ` Thomas Graf 0 siblings, 0 replies; 18+ messages in thread From: Thomas Graf @ 2007-03-20 18:15 UTC (permalink / raw) To: Patrick McHardy; +Cc: Linux Netdev List, David S. Miller, lartc * 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, ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LARTC] Re: [NET]: Fix fib_rules compatibility breakage 2007-03-20 18:15 ` Thomas Graf @ 2007-03-20 19:58 ` Patrick McHardy -1 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 19:58 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc 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> _______________________________________________ LARTC mailing list LARTC@mailman.ds9a.nl http://mailman.ds9a.nl/cgi-bin/mailman/listinfo/lartc ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [NET]: Fix fib_rules compatibility breakage @ 2007-03-20 19:58 ` Patrick McHardy 0 siblings, 0 replies; 18+ messages in thread From: Patrick McHardy @ 2007-03-20 19:58 UTC (permalink / raw) To: Thomas Graf; +Cc: Linux Netdev List, David S. Miller, lartc 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [NET]: Fix fib_rules compatibility breakage 2007-03-20 19:58 ` Patrick McHardy (?) @ 2007-03-24 19:48 ` David Miller -1 siblings, 0 replies; 18+ messages in thread From: David Miller @ 2007-03-24 19:48 UTC (permalink / raw) To: kaber; +Cc: tgraf, netdev, lartc 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2007-03-24 21:34 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-03-19 3:46 [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result Luciano Ruete 2007-03-19 5:54 ` Patrick McHardy 2007-03-19 5:54 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy 2007-03-19 15:25 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result Thomas Graf 2007-03-19 15:25 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Thomas Graf 2007-03-20 6:19 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result Patrick McHardy 2007-03-20 6:19 ` [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy 2007-03-20 6:42 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result Patrick McHardy 2007-03-20 6:42 ` [LARTC] [BUG?] ip ru flush && RTNETLINK answers: Numerical result out of range Patrick McHardy 2007-03-20 16:40 ` [LARTC] [NET]: Fix fib_rules compatibility breakage Thomas Graf 2007-03-20 16:40 ` Thomas Graf 2007-03-20 16:59 ` [LARTC] " Patrick McHardy 2007-03-20 16:59 ` Patrick McHardy 2007-03-20 18:15 ` [LARTC] " Thomas Graf 2007-03-20 18:15 ` Thomas Graf 2007-03-20 19:58 ` [LARTC] " Patrick McHardy 2007-03-20 19:58 ` Patrick McHardy 2007-03-24 19:48 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.