From mboxrd@z Thu Jan 1 00:00:00 1970 From: Edward Cree Subject: [PATCH net-next 1/3] net: core: fix uses-after-free in list processing Date: Fri, 6 Jul 2018 18:06:58 +0100 Message-ID: <102c77fb-bede-3520-6600-e94ac4a0cff4@solarflare.com> References: <3c007026-e27a-6ac9-4a29-9746840e24ed@solarflare.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: To: Return-path: Received: from dispatch1-us1.ppe-hosted.com ([148.163.129.52]:42934 "EHLO dispatch1-us1.ppe-hosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932910AbeGFRHD (ORCPT ); Fri, 6 Jul 2018 13:07:03 -0400 In-Reply-To: <3c007026-e27a-6ac9-4a29-9746840e24ed@solarflare.com> Content-Language: en-GB Sender: netdev-owner@vger.kernel.org List-ID: In netif_receive_skb_list_internal(), all of skb_defer_rx_timestamp(), do_xdp_generic() and enqueue_to_backlog() can lead to kfree(skb). Thus, we cannot wait until after they return to remove the skb from the list; instead, we remove it first and, in the pass case, add it to a sublist afterwards. In the case of enqueue_to_backlog() we have already decided not to pass when we call the function, so we do not need a sublist. Fixes: 7da517a3bc52 ("net: core: Another step of skb receive list processing") Reported-by: Dan Carpenter Signed-off-by: Edward Cree --- net/core/dev.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 89825c1eccdc..ce4583564e00 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4982,25 +4982,30 @@ static void netif_receive_skb_list_internal(struct list_head *head) { struct bpf_prog *xdp_prog = NULL; struct sk_buff *skb, *next; + struct list_head sublist; + INIT_LIST_HEAD(&sublist); list_for_each_entry_safe(skb, next, head, list) { net_timestamp_check(netdev_tstamp_prequeue, skb); - if (skb_defer_rx_timestamp(skb)) - /* Handled, remove from list */ - list_del(&skb->list); + list_del(&skb->list); + if (!skb_defer_rx_timestamp(skb)) + list_add_tail(&skb->list, &sublist); } + list_splice_init(&sublist, head); if (static_branch_unlikely(&generic_xdp_needed_key)) { preempt_disable(); rcu_read_lock(); list_for_each_entry_safe(skb, next, head, list) { xdp_prog = rcu_dereference(skb->dev->xdp_prog); - if (do_xdp_generic(xdp_prog, skb) != XDP_PASS) - /* Dropped, remove from list */ - list_del(&skb->list); + list_del(&skb->list); + if (do_xdp_generic(xdp_prog, skb) == XDP_PASS) + list_add_tail(&skb->list, &sublist); } rcu_read_unlock(); preempt_enable(); + /* Put passed packets back on main list */ + list_splice_init(&sublist, head); } rcu_read_lock(); @@ -5011,9 +5016,9 @@ static void netif_receive_skb_list_internal(struct list_head *head) int cpu = get_rps_cpu(skb->dev, skb, &rflow); if (cpu >= 0) { - enqueue_to_backlog(skb, cpu, &rflow->last_qtail); - /* Handled, remove from list */ + /* Will be handled, remove from list */ list_del(&skb->list); + enqueue_to_backlog(skb, cpu, &rflow->last_qtail); } } }