netdev - Re: [BUG] eth_type_trans() can access stale data

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:	Wed, 15 Jan 2014 00:21:09 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [BUG] eth_type_trans() can access stale data

On Tue, 2014-01-14 at 15:52 -0800, Eric Dumazet wrote:
> Following code :
> 
> if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF))
> 	return htons(ETH_P_802_3);
> 
> expects the additional bytes are in skb->head
>
> I do not think all eth_type_trans() are ready for a pskb_may_pull()
> (I am too lazy to perform a whole check)
> 
> Would following workaround be OK ?

It seems to be an improvement, as LLC packets will no longer falsely be
marked as Netware raw 802.3.  You don't fix errors in the other
direction, which should be more common, but the lack of bug reports
about that suggests that no-one is using the old Netware protocol any
more, or they're being very careful about which drivers they use.

Ben.

> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 8f032bae60ad..d95403098f09 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>  	 *      layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
>  	 *      won't work for fault tolerant netware but does for the rest.
>  	 */
> -	if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF))
> +	if (unlikely(skb_headlen(skb) >= 2 &&
> +		     *(unsigned short *)(skb->data) == 0xFFFF))
>  		return htons(ETH_P_802_3);
>  
>  	/*



-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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