netdev - Re: [PATCH v3] ipv6: Fix protocol resubmission

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: <557854BB.1090605@linux-ipv6.org>
Date:	Thu, 11 Jun 2015 00:16:11 +0900
From:	YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>
To:	Josh Hunt <johunt@...mai.com>, Hajime Tazaki <thehajime@...il.com>,
	davem@...emloft.net
CC:	kuznet@....inr.ac.ru, jmorris@...ei.org, kaber@...sh.net,
	netdev@...r.kernel.org, sergei.shtylyov@...entembedded.com,
	tom@...bertland.com
Subject: Re: [PATCH v3] ipv6: Fix protocol resubmission

Hi,

Josh Hunt wrote:
> On 06/09/2015 11:24 PM, Hajime Tazaki wrote:
>>
>> Hello Josh, Dave,
>>
>> my mobile ipv6 test on libos failed with this commit.
>>
>> This commit makes a destination option header handling (i.e.,
>> ipprot->handler == ipv6_destopt_rcv) failed since
>> ipv6_destopt_rcv() seems to return a positive value to
>> indicate to goto resubmission label.
>>
>> I will look for more detail.
>>
>> -- Hajime
> 
> Hajime
> 
> Thanks for the report. I mentioned in an earlier post this might be a problem.
> 
> Dave, what if we restore the old behavior, but add a new label to handle the case where the decapsulating protocol returns the nexthdr value? Allowing for migration over to this method over time. I've pasted in a patch doing so below.

I think it is insufficient because IPv6 stack already uses
positive value, 0 and negative values.

> 
> The other solution I guess is to change how the udp handler works, but I was hoping to keep it behaving the same as v4.

xfrm returns different value for IPv4 and IPv6, for example,
so udp can do in the same way.  And we can use the fact that
the size of next header field is 8-bit.


> 
> Josh
> 
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index 41a73da..a4fab24 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -212,13 +212,13 @@ static int ip6_input_finish(struct sock *sk, struct sk_buff *skb)
>          */
> 
>         rcu_read_lock();
> +resubmit:
>         idev = ip6_dst_idev(skb_dst(skb));
>         if (!pskb_pull(skb, skb_transport_offset(skb)))
>                 goto discard;
>         nhoff = IP6CB(skb)->nhoff;
>         nexthdr = skb_network_header(skb)[nhoff];
> -
> -resubmit:
> +resubmit_nexthdr:
>         raw = raw6_local_deliver(skb, nexthdr);
>         ipprot = rcu_dereference(inet6_protos[nexthdr]);
>         if (ipprot) {
> @@ -246,9 +246,11 @@ resubmit:
>                         goto discard;
> 
>                 ret = ipprot->handler(skb);
> -               if (ret < 0) {
> -                       nexthdr = -ret;
> +               if (ret > 0) {
>                         goto resubmit;
> +               } else if (ret < 0) {
> +                       nexthdr = -ret;
> +                       goto resubmit_nexthdr;
>                 } else if (ret == 0) {
>                         IP6_INC_STATS_BH(net, idev, IPSTATS_MIB_INDELIVERS);
>                 }

--
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