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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Apr 2017 09:07:12 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     James Hughes <james.hughes@...pberrypi.org>
Cc:     netdev@...r.kernel.org
Subject: Re: Use of skb_unclone in drivers

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.