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

Reply via email to