Hi, I still seem to have serious problems with my mailer. Despite numerous resends, I still haven't seen my reply on netdev. Hopefully it finally gets through, sorry for any possible duplicates.
Ville Nuorvala wrote: > YOSHIFUJI Hideaki wrote: >> Hello. > > Hello Yoshifuji-san! > >> Here's a set of changesets (on top of net-2.6.19 tree) to fix routing / >> ndisc. >> Changesets are available at: >> >> git://git.skbuff.net/gitroot/yoshfuji/net-2.6.19-20060809-polroute-fixes/ > > I'd like to comment some of the NDISC patches a bit (comments inline), > but all other changes looked good. > >> CHANGESETS >> ---------- >> >> commit 4f2956c43d77e1efbf044db305455493276fc6f2 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 16:53:52 2006 +0900 >> >> [IPV6] NDISC: Take source address into account for redirects. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > > >> --- >> commit 40ff54178bd3c5dbd80f9422e88f7539727cc4e7 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 16:53:53 2006 +0900 >> >> [IPV6] NDISC: Search over all possible rules on receipt of redirect. >> >> Split up function for finding routes for redirects. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> >> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >> index 91c9461..4650787 100644 >> --- a/net/ipv6/route.c >> +++ b/net/ipv6/route.c >> @@ -1282,19 +1282,18 @@ static int ip6_route_del(struct in6_rtms >> /* >> * Handle redirects >> */ >> -void rt6_redirect(struct in6_addr *dest, struct in6_addr *src, >> - struct in6_addr *saddr, >> - struct neighbour *neigh, u8 *lladdr, int on_link) >> +struct ip6rd_flowi { >> + struct flowi fl; >> + struct in6_addr gateway; >> +}; >> + >> +static struct rt6_info *__ip6_route_redirect(struct fib6_table *table, >> + struct flowi *fl, >> + int flags) >> { >> - struct rt6_info *rt, *nrt = NULL; >> + struct ip6rd_flowi *rdfl = (struct ip6rd_flowi *)fl; >> + struct rt6_info *rt; >> struct fib6_node *fn; >> - struct fib6_table *table; >> - struct netevent_redirect netevent; >> - >> - /* TODO: Very lazy, might need to check all tables */ >> - table = fib6_get_table(RT6_TABLE_MAIN); >> - if (table == NULL) >> - return; >> >> /* >> * Get the "current" route for this destination and >> @@ -1308,7 +1307,7 @@ void rt6_redirect(struct in6_addr *dest, >> */ >> >> read_lock_bh(&table->tb6_lock); >> - fn = fib6_lookup(&table->tb6_root, dest, src); >> + fn = fib6_lookup(&table->tb6_root, &fl->fl6_dst, &fl->fl6_src); >> restart: >> for (rt = fn->leaf; rt; rt = rt->u.next) { >> /* >> @@ -1323,29 +1322,67 @@ restart: >> continue; >> if (!(rt->rt6i_flags & RTF_GATEWAY)) >> continue; >> - if (neigh->dev != rt->rt6i_dev) >> + if (fl->oif != rt->rt6i_dev->ifindex) >> continue; >> - if (!ipv6_addr_equal(saddr, &rt->rt6i_gateway)) >> + if (!ipv6_addr_equal(&rdfl->gateway, &rt->rt6i_gateway)) >> continue; >> break; >> } >> - if (rt) >> - dst_hold(&rt->u.dst); >> - else if (rt6_need_strict(dest)) { >> - while ((fn = fn->parent) != NULL) { >> - if (fn->fn_flags & RTN_ROOT) >> - break; >> - if (fn->fn_flags & RTN_RTINFO) >> - goto restart; >> + >> + if (!rt) { >> + if (rt6_need_strict(&fl->fl6_dst)) { >> + while ((fn = fn->parent) != NULL) { >> + if (fn->fn_flags & RTN_ROOT) >> + break; >> + if (fn->fn_flags & RTN_RTINFO) >> + goto restart; >> + } >> } >> + rt = &ip6_null_entry; >> } >> + dst_hold(&rt->u.dst); >> + >> read_unlock_bh(&table->tb6_lock); >> >> - if (!rt) { >> + return rt; >> +}; >> + >> +static struct rt6_info *ip6_route_redirect(struct in6_addr *dest, >> + struct in6_addr *src, >> + struct in6_addr *gateway, >> + struct net_device *dev) >> +{ >> + struct ip6rd_flowi rdfl = { >> + .fl = { >> + .oif = dev->ifindex, >> + .nl_u = { >> + .ip6_u = { >> + .daddr = *dest, >> + .saddr = *src, >> + }, >> + }, >> + }, >> + .gateway = *gateway, >> + }; >> + int flags = rt6_need_strict(dest) ? RT6_F_STRICT : 0; >> + >> + return (struct rt6_info *)fib6_rule_lookup((struct flowi *)&rdfl, >> flags, __ip6_route_redirect); >> +} >> + >> +void rt6_redirect(struct in6_addr *dest, struct in6_addr *src, >> + struct in6_addr *saddr, >> + struct neighbour *neigh, u8 *lladdr, int on_link) >> +{ >> + struct rt6_info *rt, *nrt = NULL; >> + struct netevent_redirect netevent; >> + >> + rt = ip6_route_redirect(dest, src, saddr, neigh->dev); >> + >> + if (rt == &ip6_null_entry) { >> if (net_ratelimit()) >> printk(KERN_DEBUG "rt6_redirect: source isn't a valid >> nexthop " >> "for redirect target\n"); >> - return; >> + goto out; >> } >> >> /* > > This might work correctly, but I'd like to make sure. If it works, I'd > like to know this is by choice and not by chance. > > As ip6_route_redirect can't possibly know the (possible) incoming > interface of the original redirected packet it can't set fl.iif. This > means the the route lookup result might differ from the original route > lookup. However, a situation like this doesn't arise unless the node > functions as a router. > > According to the RFC 2461 a router MUST NOT change its routing behavior > as the result of an ICMPv6 redirect, i.e. it has to ignore the message. > In reality things are not always as clear cut as this. The Mobile Router > defined in RFC 3963 acts as a router between its virtual tunnel > interface and its ingress interface, but in practice acts as a host on > its egress interface. That being said, I think your code works ok even > in this case, but I'd have to test this to make sure. > >> --- >> commit e0ad64d5b44179ea1296d737dec23279c72c9636 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:08:33 2006 +0900 >> >> [IPV6] NDISC: Allow redirects from other interfaces if it is not strict. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> >> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >> index 4650787..1698fec 100644 >> --- a/net/ipv6/route.c >> +++ b/net/ipv6/route.c >> @@ -1322,7 +1322,7 @@ restart: >> continue; >> if (!(rt->rt6i_flags & RTF_GATEWAY)) >> continue; >> - if (fl->oif != rt->rt6i_dev->ifindex) >> + if ((flags & RT6_F_STRICT) && fl->oif != rt->rt6i_dev->ifindex) >> continue; >> if (!ipv6_addr_equal(&rdfl->gateway, &rt->rt6i_gateway)) >> continue; >> > > Is this absolutely safe? Doesn't this enable a malicious node on another > link to make a bogus redirect if it uses same link-local source address > as the real router on the other link. Keep in mind that the RT6_F_STRICT > flag is set based on the destination of the original redirected packet > and doesn't in any way depend on the router or source address. > > Without SEND, similar attacks are of course possible when the attacker > is on the same link as the router, but I suspect we are opening up a new > hole here. > >> --- >> commit 67539e5824106359507ea462035fa8bb57c20d4c >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:08:41 2006 +0900 >> >> [IPV6] NDISC: Initialize fl with outbound interface to lookup rules >> properly. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 8fc359533dbc3962f32ef2cf39f1e0bf1f5be33b >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:09:13 2006 +0900 >> >> [IPV6] ROUTE: Introduce a helper to check route validity. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Acked-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 25ee62e8a25adfbb2d64c4b54a759d4fbf5be9d8 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:14:39 2006 +0900 >> >> [IPV6]: Cache source address as well in ipv6_pinfo{}. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 61391ed3da4ba78353febdb69e9faa9832479425 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:17:47 2006 +0900 >> >> [IPV6] ROUTE: Make sure we have fn->leaf when adding a node on subtree. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 7c191ae22dee4465fffd8603429385fbea518faa >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:18:06 2006 +0900 >> >> [IPV6] ROUTE: Prune clones from main tree as well. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 7e7d663f87c72805f68317d402107e81ff309c0d >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:18:31 2006 +0900 >> >> [IPV6] ROUTE: Fix looking up a route on subtree. >> >> Even on RTN_ROOT node, we need to process its subtree first. >> Fix NULL pointer dereference in fib6_locate(). >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 1b5fab0cbe09e9aa00ff1c7f13aa204aca8c4b29 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:18:56 2006 +0900 >> >> [IPV6] ROUTE: Make sure we do not exceed args in fib6_lookup_1(). >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Acked-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit eec98f168a438781c133270dfdf456b345fd48d2 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:19:15 2006 +0900 >> >> [IPV6] ROUTE: Allow searching subtree only. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> >> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c >> index b24b6a4..3d45a44 100644 >> --- a/net/ipv6/ip6_fib.c >> +++ b/net/ipv6/ip6_fib.c >> @@ -753,7 +753,7 @@ #endif >> } >> }; >> >> - fn = fib6_lookup_1(root, args); >> + fn = fib6_lookup_1(root, daddr ? args : args + 1); >> >> if (fn == NULL || fn->fn_flags & RTN_TL_ROOT) >> fn = root; >> > > Acked-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 450a6aa5da9a8ffba9a9e462183b0ab76bbfd40c >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:19:37 2006 +0900 >> >> [IPV6] ROUTE: Put SUBTREE() as FIB6_SUBTREE() into ip6_fib.h for future >> use. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit 09aa35ff359e520abb11b6f71deb21f79da30a52 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:29:18 2006 +0900 >> >> [IPV6] ROUTE: Search subtree when backtracking. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED]> > >> --- >> commit a75bc4c27c306402d721310e92060969e6e5a031 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:29:33 2006 +0900 >> >> [IPV6] ROUTE: Purge clones on other trees when deleting a route. >> >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED] > >> --- >> commit 5cb675bce7549177c09ad42e48e07a59df5e0c3f >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:33:35 2006 +0900 >> >> [IPV6] NDISC: Search subtrees when backtracking on receipt of redirects. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Acked-by: Ville Nuorvala <[EMAIL PROTECTED] > >> --- >> commit 9458f9452e16b5ef6c0c70e0e134513a5f07632b >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 17:37:16 2006 +0900 >> >> [IPV6] KCONFIG: Add subtrees support. >> >> This is for developers only. >> Based on MIPL2 kernel patch. >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Signed-off-by: Ville Nuorvala <[EMAIL PROTECTED] > >> --- >> commit 218aaaf16e581fce753fcf581d40915da1e23b06 >> Author: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> >> Date: Wed Aug 9 18:05:02 2006 +0900 >> >> [IPV6] ROUTE: Unify RT6_F_xxx and RT6_SELECT_F_xxx flags >> >> Unify RT6_F_xxx and RT6_SELECT_F_xxx flags into >> RT6_LOOKUP_F_xxx flags, and put them into ip6_route.h >> >> Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]> > > Acked-by: Ville Nuorvala <[EMAIL PROTECTED] > > Regards, > Ville > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html