netdev - Re: [net PATCH] net: Copy inner L3 and L4 headers as unaligned on GRE TEB

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: <CAKgT0Ud--ruzbDWR2dK+2LY6uMXo8iRSE_6cwAw0eb0f4eC3cQ@mail.gmail.com>
Date:	Tue, 9 Feb 2016 16:36:06 +0100
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Tom Herbert <tom@...bertland.com>
Cc:	Alexander Duyck <aduyck@...antis.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [net PATCH] net: Copy inner L3 and L4 headers as unaligned on GRE TEB

On Tue, Feb 9, 2016 at 3:33 PM, Tom Herbert <tom@...bertland.com> wrote:
> On Tue, Feb 9, 2016 at 3:14 PM, Alexander Duyck <aduyck@...antis.com> wrote:
>> This patch corrects the unaligned accesses seen on GRE TEB tunnels when
>> generating hash keys.  Specifically what this patch does is make it so that
>> we force the use of skb_copy_bits when the GRE inner headers will be
>> unaligned due to NET_IP_ALIGNED being a non-zero value.
>>
>> Signed-off-by: Alexander Duyck <aduyck@...antis.com>
>> ---
>>
>> I don't have the ability to test it but this should fix flow dissector for
>> GRE TEB tunnels traffic seen on architectures that require network and
>> transport headers to be 4 byte aligned.
>>
>>  net/core/flow_dissector.c |    7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 699b2c415cb0..9c181ba7263e 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -402,6 +402,13 @@ ip_proto_again:
>>                                 goto out_bad;
>>                         proto = eth->h_proto;
>>                         nhoff += sizeof(*eth);
>> +
>> +                       /* Cap headers that we access via pointers at the
>> +                        * end of the Ethernet header as our maximum alignment
>> +                        * at that point is only 2 bytes.
>> +                        */
>> +                       if (NET_IP_ALIGN)
>> +                               hlen = nhoff;
>
> I think this should be:
>
> if (NET_IP_ALIGN)
>      goto out_good;
>

That is no good since we already updated proto with the inner header
protocol value.  I would prefer to parse the entire header and just
keep the behavior consistent between IP aligned and DMA aligned
systems.

The only change in behavior is that the reported header length from
the function eth_get_headlen will only pull to the end of the Ethernet
header since we are now only reporting the aligned IP header length.

Otherwise if we need to exit we should probably exit with "goto out_bad"

- Alex