netdev - Re: Use of skb_unclone in drivers

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: <CAE_XsMLghPZTe69-eUF322U53gZcUfXzu-0VScf7yNCrLkgOBA@mail.gmail.com>
Date:   Tue, 18 Apr 2017 09:34:10 +0100
From:   James Hughes <james.hughes@...pberrypi.org>
To:     netdev@...r.kernel.org
Subject: Re: Use of skb_unclone in drivers

Thanks,

Quick check on that function - very similar to unclone which we know
ameliorates the issue, just has the headroom parameter rather than
priority. So clearly the right way to go. Just one more question I
hope - where is the appropriate  place for the skb_cow_head call in
the driver - at the main entry point, or closer or in the function(s)
that may alter the header itself?

James

(Apologies to Eric for originally sending him this message rather than
to netdev - was doing it all from a phone and screwed it up)

On 17 April 2017 at 17:07, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Mon, 2017-04-17 at 16:02 +0100, James Hughes wrote:
>> Netdevs,
>>
>> We have recently got to the bottom of an issue which we have been
>> encountering on a Raspberry Pi being used as an access point, and we
>> need a bit of advice on the correct way of fixing the issue.
>>
>> The set up is a Raspberry Pi 3 running hostapd on its inbuilt wireless
>> adaptor (Brcm43438 ), bridged to the built in ethernet adaptor
>> (smsc9514). Using the standard drivers for these devices as of 4.9
>> (looking at 4.11, no changes noted that would affect the issue. I will
>> be trying the latest kernel once I get back in to the office).
>>
>> We were encountering an error in the Brcm Wireless driver that after
>> investigation was a skb_buff being corrupted by an action in the smsc
>> Ethernet driver. Further digging shows that the bridge was cloning an
>> incoming skb from the Ethernet, then sending it out to both the
>> Ethernet and wlan ports. This mean that both the Ethernet driver and
>> the wireless driver were looking at the same physical data, and one or
>> both were altering that data in different ways (varied headers) ,
>> which mean that headers were corrupted. This was only happening on
>> broadcast packets.
>>
>> We can fix the issue by, in the drivers, using skb_unclone in
>> appropriate places in the driver.
>>
>> So now to the questions. Is adding the unclone to the drivers the
>> correct way of dealing with this issue? Examining other drivers shows
>> that unclone is not particularly common, presumably in many cases,
>> where the driver does not alter the skb, it doesn't matter. However,
>> in any case where a driver may add header information to the skb (as
>> is the case with the Brcm wireless driver), when the incoming skb has
>> been cloned, the results must surely be undetermined unless an unclone
>> is performed.
>>
>> Or, is the bridging code making a mistake by cloning the skb and
>> passing it to multiple recipients?
>>
>
>
> bridge code is fine.
>
> Problem is that some drivers lack calls to skb_cow_head() before they
> mess with headers.
>
>
>
>
>
>



-- 
James Hughes
Principal Software Engineer,
Raspberry Pi (Trading) Ltd