netdev - Re: [net PATCH] flow_dissector: Fix unaligned access in __skb_flow_dissector when used by eth_get

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]

Date:	Sat, 30 Jan 2016 09:43:00 -0800
From:	Tom Herbert <tom@...bertland.com>
To:	Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Alexander Duyck <aduyck@...antis.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [net PATCH] flow_dissector: Fix unaligned access in
 __skb_flow_dissector when used by eth_get_headlen

On Sat, Jan 30, 2016 at 8:17 AM, Sowmini Varadhan
<sowmini.varadhan@...cle.com> wrote:
> On (01/29/16 19:23), Eric Dumazet wrote:
>> BTW, even a memcpy(&key_addrs->v4addrs, &iph->saddr, 8) could crash, as
>> the compiler can certainly assume src and dst are 4 bytes aligned, and
>> could use word accesses when inlining memcpy() even on Sparc.
>>
>> Apparently the compiler used by Sowmini is gentle.
>
> One more subtlety that I missed until now..
>
> eth_get_headlen passes in flow_keys_buf_dissector (NOT flow_keys_dissector!)
>
> So FLOW_DISSECTOR_KEY_IPV4_ADDRS is not set, and this helps to dodge
> the unaligned iph->saddr access.
>
> But as others have pointed out, much of this code is brittle
> because it's accessing the data before the driver has had a chance
> to align things. The page_offset initialization of NET_IP_ALIGN,
> with all its weaknesses, at least matches (in principle) the prescription
> used for the xmit path.
>
That is not the only case, If a GRE TEB packet is ever received and
flow dissector is called for any reason (like skb_get_hash) there's
going to be problems-- and this doesn't require GRE to even be
configured on the host.

I have a patch that changes the 32-bit accesses in flow_dissector to
use get_unaligned_be32. I don't have access to any machines that care
about alignment (only x86). Do you know if there is an alternate way
to test this other than running on architecture like Sparc?

Thanks,
Tom

> --Sowmini
>