netdev - Re: [PATCH net] atl1c: reserve min skb headroom

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: <65cc6398-8e9d-7b7d-f601-10fff59be85d@gmail.com>
Date:   Fri, 20 Jul 2018 10:48:29 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Florian Westphal <fw@...len.de>, netdev@...r.kernel.org
Cc:     eric.dumazet@...il.com
Subject: Re: [PATCH net] atl1c: reserve min skb headroom



On 07/20/2018 10:30 AM, Florian Westphal wrote:
> Got crash report with following backtrace:
> BUG: unable to handle kernel paging request at ffff8801869daffe
> RIP: 0010:[<ffffffff816429c4>]  [<ffffffff816429c4>] ip6_finish_output2+0x394/0x4c0
> RSP: 0018:ffff880186c83a98  EFLAGS: 00010283
> RAX: ffff8801869db00e ...
>   [<ffffffff81644cdc>] ip6_finish_output+0x8c/0xf0
>   [<ffffffff81644d97>] ip6_output+0x57/0x100
>   [<ffffffff81643dc9>] ip6_forward+0x4b9/0x840
>   [<ffffffff81645566>] ip6_rcv_finish+0x66/0xc0
>   [<ffffffff81645db9>] ipv6_rcv+0x319/0x530
>   [<ffffffff815892ac>] netif_receive_skb+0x1c/0x70
>   [<ffffffffc0060bec>] atl1c_clean+0x1ec/0x310 [atl1c]
>   ...
> 
> The bad access is in neigh_hh_output(), at skb->data - 16 (HH_DATA_MOD).
> atl1c driver provided skb with no headroom, so 14 bytes (ethernet
> header) got pulled, but then 16 are copied.
> 
> Reserve NET_SKB_PAD bytes headroom, like netdev_alloc_skb().
> 
> Compile tested only; I lack hardware.
> 
> Fixes: 7b7017642199 ("atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring")
> Signed-off-by: Florian Westphal <fw@...len.de>
> ---
> diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> index 94270f654b3b..7087b88550db 100644
> --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> @@ -1686,6 +1686,7 @@ static struct sk_buff *atl1c_alloc_skb(struct atl1c_adapter *adapter)
>  	skb = build_skb(page_address(page) + adapter->rx_page_offset,
>  			adapter->rx_frag_size);
>  	if (likely(skb)) {
> +		skb_reserve(skb, NET_SKB_PAD);
>  		adapter->rx_page_offset += adapter->rx_frag_size;
>  		if (adapter->rx_page_offset >= PAGE_SIZE)
>  			adapter->rx_page = NULL;
> 

Yes, it is interesting IPv4 has code to deal with that( in ip_finish_output2()),
not IPv6 :/

Reviewed-by: Eric Dumazet <edumazet@...gle.com>