netdev - Re: [PATCH net] net: use skb_list_del_init() to remove from RX sublists

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] [day] [month] [year] [list]

Message-Id: <20181205.162404.1991489633644733479.davem@davemloft.net>
Date:   Wed, 05 Dec 2018 16:24:04 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     ecree@...arflare.com
Cc:     linux-net-drivers@...arflare.com, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: use skb_list_del_init() to remove from RX
 sublists

From: Edward Cree <ecree@...arflare.com>
Date: Tue, 4 Dec 2018 17:37:57 +0000

> list_del() leaves the skb->next pointer poisoned, which can then lead to
>  a crash in e.g. OVS forwarding.  For example, setting up an OVS VXLAN
>  forwarding bridge on sfc as per:
 ...
> So, in all listified-receive handling, instead pull skbs off the lists with
>  skb_list_del_init().
> 
> Fixes: 9af86f933894 ("net: core: fix use-after-free in __netif_receive_skb_list_core")
> Fixes: 7da517a3bc52 ("net: core: Another step of skb receive list processing")
> Fixes: a4ca8b7df73c ("net: ipv4: fix drop handling in ip_list_rcv() and ip_list_rcv_finish()")
> Fixes: d8269e2cbf90 ("net: ipv6: listify ipv6_rcv() and ip6_rcv_finish()")
> Signed-off-by: Edward Cree <ecree@...arflare.com>

Applied and queued up for -stable

> I'm not sure if these are the right Fixes tags, or if I should instead be
>  fingering some commit that made dev_hard_start_xmit() more sensitive to
>  skb->next.
> Also, I only saw a crash from the list_del() in __netif_receive_skb_list_core()
>  but I converted all of them in the listified RX path, in case any others
>  have similar ways to escape into paths that care about skb->next.

I think we should use skb_list_del_init() on in all cases skb->list except
where we immediately queue it onto another list in a trivially auditable
way.

Therefore I think what you did is the way to go.

Thanks.