netdev - Re: [REGRESSION,BISECTED] MIPv6 support broken by f4f914b58019f0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C00047D.3010404@hp.com>
Date:	Fri, 28 May 2010 13:59:25 -0400
From:	Brian Haley <brian.haley@...com>
To:	Arnaud Ebalard <arno@...isbad.org>
CC:	Scott C Otto <otts@...atel-lucent.com>,
	David Miller <davem@...emloft.net>,
	YOSHIFUJI Hideaki / 吉藤英明 
	<yoshfuji@...ux-ipv6.org>, Jiri Olsa <jolsa@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [REGRESSION,BISECTED] MIPv6 support broken by f4f914b58019f0

On 05/28/2010 04:51 AM, Arnaud Ebalard wrote:
>>> The below might actually be what was actually intended, triggering
>>> on what the user forced, rather than assuming all callers require
>>> strict behavior.
>>>
>>> -Brian
>>>
>>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 294cbe8..252d761 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -814,7 +814,7 @@ struct dst_entry * ip6_route_output(struct net *net, struct sock *sk,
>>>  {
>>>  	int flags = 0;
>>>  
>>> -	if (fl->oif || rt6_need_strict(&fl->fl6_dst))
>>> +	if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(&fl->fl6_dst))
>>>  		flags |= RT6_LOOKUP_F_IFACE;
>>>  
>>>  	if (!ipv6_addr_any(&fl->fl6_src))
> 
> Brian, I tested the patch on my Mobile Node: it fixes the regression. I
> also updated the kernel on my Home Agent to a 2.6.34 with that fix and
> everything works as expected. *For that aspect* and fwiw, you get my
> 
> Tested-by: Arnaud Ebalard <arno@...isbad.org>

Ok, thanks for testing, I'll send out an updated patch, with the caveat
there still might be a DCCP regression, see below.

> For the SO_BINDTODEVICE aspect, I don't have code at hand to test if the
> fix works as expected. We should also double check that this will not
> break other paths which use the sk->sk_bound_dev_if with a different
> semantic:

> net/ieee802154/raw.c:   sk->sk_bound_dev_if = dev->ifindex;

Isn't AF_INET6, OK.

> net/core/sock.c:        sk->sk_bound_dev_if = index;

The actual setsockopt() call, OK.

> net/ipv6/datagram.c:    sk->sk_bound_dev_if = usin->sin6_scope_id;
> net/ipv6/datagram.c:    sk->sk_bound_dev_if = np->mcast_oif;
> net/ipv6/af_inet6.c:    sk->sk_bound_dev_if = addr->sin6_scope_id;
> net/ipv6/tcp_ipv6.c:    sk->sk_bound_dev_if = usin->sin6_scope_id;
> net/ipv6/tcp_ipv6.c:    newsk->sk_bound_dev_if = treq->iif;
> net/ipv6/raw.c:         sk->sk_bound_dev_if = addr->sin6_scope_id;

This are all in link-local or multicast code, caller passing-in scopeid,
would trigger rt6_need_strict() regardless, so are OK.

> net/sctp/socket.c:      newsk->sk_bound_dev_if = sk->sk_bound_dev_if;

SCTP peeling-off a socket, cloning parent, OK.

> net/ipv4/ip_output.c:   sk->sk_bound_dev_if = arg->bound_dev_if;
> net/ipv4/udp.c:         sk->sk_bound_dev_if = 0;

IPv4, shouldn't be affected by an IPv6 route change, OK.

> net/dccp/ipv6.c:        newsk->sk_bound_dev_if = ireq6->iif;

This is the only mystery in this list.  Looks like the DCCP accept()
codepath, and it's getting bound to the interface the request was
received on.  I'm not sure if the intention here was to force this
strict behavior or not.

> net/dccp/ipv6.c:        sk->sk_bound_dev_if = usin->sin6_scope_id;

In link-local code, caller passing-in scopeid, would trigger
rt6_need_strict() regardless, so OK.

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