diff options
| author | Edward Cree <ecree@solarflare.com> | 2018-07-09 13:09:54 -0400 |
|---|---|---|
| committer | David S. Miller <davem@davemloft.net> | 2018-07-09 17:55:53 -0400 |
| commit | 8c057efaebb557b60ba514b5e39e8000a1eab0f1 (patch) | |
| tree | 9e179eec6b8563653162dc8c5d933b454e21dcfc /net/core/dev.c | |
| parent | c47078d6a33fd78d882200cdaacbcfcd63318234 (diff) | |
net: core: fix uses-after-free in list processing
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 <dan.carpenter@oracle.com>
Signed-off-by: Edward Cree <ecree@solarflare.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/core/dev.c')
| -rw-r--r-- | net/core/dev.c | 21 |
1 files 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) | |||
| 4982 | { | 4982 | { |
| 4983 | struct bpf_prog *xdp_prog = NULL; | 4983 | struct bpf_prog *xdp_prog = NULL; |
| 4984 | struct sk_buff *skb, *next; | 4984 | struct sk_buff *skb, *next; |
| 4985 | struct list_head sublist; | ||
| 4985 | 4986 | ||
| 4987 | INIT_LIST_HEAD(&sublist); | ||
| 4986 | list_for_each_entry_safe(skb, next, head, list) { | 4988 | list_for_each_entry_safe(skb, next, head, list) { |
| 4987 | net_timestamp_check(netdev_tstamp_prequeue, skb); | 4989 | net_timestamp_check(netdev_tstamp_prequeue, skb); |
| 4988 | if (skb_defer_rx_timestamp(skb)) | 4990 | list_del(&skb->list); |
| 4989 | /* Handled, remove from list */ | 4991 | if (!skb_defer_rx_timestamp(skb)) |
| 4990 | list_del(&skb->list); | 4992 | list_add_tail(&skb->list, &sublist); |
| 4991 | } | 4993 | } |
| 4994 | list_splice_init(&sublist, head); | ||
| 4992 | 4995 | ||
| 4993 | if (static_branch_unlikely(&generic_xdp_needed_key)) { | 4996 | if (static_branch_unlikely(&generic_xdp_needed_key)) { |
| 4994 | preempt_disable(); | 4997 | preempt_disable(); |
| 4995 | rcu_read_lock(); | 4998 | rcu_read_lock(); |
| 4996 | list_for_each_entry_safe(skb, next, head, list) { | 4999 | list_for_each_entry_safe(skb, next, head, list) { |
| 4997 | xdp_prog = rcu_dereference(skb->dev->xdp_prog); | 5000 | xdp_prog = rcu_dereference(skb->dev->xdp_prog); |
| 4998 | if (do_xdp_generic(xdp_prog, skb) != XDP_PASS) | 5001 | list_del(&skb->list); |
| 4999 | /* Dropped, remove from list */ | 5002 | if (do_xdp_generic(xdp_prog, skb) == XDP_PASS) |
| 5000 | list_del(&skb->list); | 5003 | list_add_tail(&skb->list, &sublist); |
| 5001 | } | 5004 | } |
| 5002 | rcu_read_unlock(); | 5005 | rcu_read_unlock(); |
| 5003 | preempt_enable(); | 5006 | preempt_enable(); |
| 5007 | /* Put passed packets back on main list */ | ||
| 5008 | list_splice_init(&sublist, head); | ||
| 5004 | } | 5009 | } |
| 5005 | 5010 | ||
| 5006 | rcu_read_lock(); | 5011 | rcu_read_lock(); |
| @@ -5011,9 +5016,9 @@ static void netif_receive_skb_list_internal(struct list_head *head) | |||
| 5011 | int cpu = get_rps_cpu(skb->dev, skb, &rflow); | 5016 | int cpu = get_rps_cpu(skb->dev, skb, &rflow); |
| 5012 | 5017 | ||
| 5013 | if (cpu >= 0) { | 5018 | if (cpu >= 0) { |
| 5014 | enqueue_to_backlog(skb, cpu, &rflow->last_qtail); | 5019 | /* Will be handled, remove from list */ |
| 5015 | /* Handled, remove from list */ | ||
| 5016 | list_del(&skb->list); | 5020 | list_del(&skb->list); |
| 5021 | enqueue_to_backlog(skb, cpu, &rflow->last_qtail); | ||
| 5017 | } | 5022 | } |
| 5018 | } | 5023 | } |
| 5019 | } | 5024 | } |
