netdev - Re: [PATCH v2] 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: <5568DD83.8020600@akamai.com>
Date:	Fri, 29 May 2015 16:43:31 -0500
From:	Josh Hunt <johunt@...mai.com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	davem@...emloft.net
CC:	netdev@...r.kernel.org, tom@...bertland.com
Subject: Re: [PATCH v2] ipv6: Fix protocol resubmission

On 05/29/2015 03:27 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 05/29/2015 11:04 PM, Josh Hunt wrote:
>
>> I came across this problem while trying to use UDP encapsulation with
>> IPv6. The
>> change below fixes that, but it was not immediately apparent if there
>> are any
>> other protocols relying on this broken behavior. FWIW the behavior
>> below now
>> matches IPv4.
>
>> Josh
>
>     Need "Signed-off-by:" line insted of this.
>
>> v2: Actually sets nexthdr so we can use it ;)
>
>     Normally goes after ---, not before.
>
>> ---
>
>> UDP encapsulation is broken on IPv6 because it expects when it returns
>> a negative
>> value that the packet will be resubmitted to the stack with the
>> handler corresponding
>> to the return value. The check currently looks for return values > 0
>> and then resubmits.
>
>> This patch fixes that check and also moves the resubmit label to take
>> advantage of
>> the return code identifying the next protocol we want to process.
>
>> Signed-off-by: Josh Hunt <johunt@...mai.com>
>> ---
>>   net/ipv6/ip6_input.c |    7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>
>> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
>> index f2e464e..e16c289 100644
>> --- a/net/ipv6/ip6_input.c
>> +++ b/net/ipv6/ip6_input.c
> [...]
>> @@ -246,9 +246,10 @@ resubmit:
>>               goto discard;
>>
>>           ret = ipprot->handler(skb);
>> -        if (ret > 0)
>> +        if (ret < 0) {
>> +            nexthdr = -ret;
>>               goto resubmit;
>> -        else if (ret == 0)
>> +        } else if (ret == 0)
>>               IP6_INC_STATS_BH(net, idev, IPSTATS_MIB_INDELIVERS);
>
>     The CodingStyle dictates that all branches must have {} if at least
> one has them.

Thanks Sergei, I appreciate the review. I will make these changes in a 
v3 after I get feedback this is the correct fix for the problem.

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