* [PATCH v2 net-next 0/2] fix use-after-free bugs in skb list processing
@ 2018-07-09 13:24 Edward Cree
2018-07-09 13:25 ` [PATCH v2 net-next 1/2] net: core: fix uses-after-free in " Edward Cree
2018-07-09 13:26 ` [PATCH v2 net-next 2/2] netfilter: fix use-after-free in NF_HOOK_LIST Edward Cree
0 siblings, 2 replies; 3+ messages in thread
From: Edward Cree @ 2018-07-09 13:24 UTC (permalink / raw)
To: davem; +Cc: netdev
A couple of bugs in skb list handling were spotted by Dan Carpenter, with
the help of Smatch; following up on them I found a couple more similar
cases. This series fixes them by changing the relevant loops to use the
dequeue-enqueue model (rather than in-place list modification).
v2: dropped patch #3 (new list.h helper), per DaveM's request.
Edward Cree (2):
net: core: fix uses-after-free in list processing
netfilter: fix use-after-free in NF_HOOK_LIST
include/linux/netfilter.h | 10 +++++++---
net/core/dev.c | 21 +++++++++++++--------
2 files changed, 20 insertions(+), 11 deletions(-)
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 net-next 1/2] net: core: fix uses-after-free in list processing
2018-07-09 13:24 [PATCH v2 net-next 0/2] fix use-after-free bugs in skb list processing Edward Cree
@ 2018-07-09 13:25 ` Edward Cree
2018-07-09 13:26 ` [PATCH v2 net-next 2/2] netfilter: fix use-after-free in NF_HOOK_LIST Edward Cree
1 sibling, 0 replies; 3+ messages in thread
From: Edward Cree @ 2018-07-09 13:25 UTC (permalink / raw)
To: davem; +Cc: netdev
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>
---
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);
}
}
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 net-next 2/2] netfilter: fix use-after-free in NF_HOOK_LIST
2018-07-09 13:24 [PATCH v2 net-next 0/2] fix use-after-free bugs in skb list processing Edward Cree
2018-07-09 13:25 ` [PATCH v2 net-next 1/2] net: core: fix uses-after-free in " Edward Cree
@ 2018-07-09 13:26 ` Edward Cree
1 sibling, 0 replies; 3+ messages in thread
From: Edward Cree @ 2018-07-09 13:26 UTC (permalink / raw)
To: davem; +Cc: netdev
nf_hook() can free the skb, so we need to remove it from the list before
calling, and add passed skbs to a sublist afterwards.
Fixes: 17266ee93984 ("net: ipv4: listified version of ip_rcv")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
include/linux/netfilter.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 5a5e0a2ab2a3..23b48de8c2e2 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -294,12 +294,16 @@ NF_HOOK_LIST(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
int (*okfn)(struct net *, struct sock *, struct sk_buff *))
{
struct sk_buff *skb, *next;
+ struct list_head sublist;
+ INIT_LIST_HEAD(&sublist);
list_for_each_entry_safe(skb, next, head, list) {
- int ret = nf_hook(pf, hook, net, sk, skb, in, out, okfn);
- if (ret != 1)
- list_del(&skb->list);
+ list_del(&skb->list);
+ if (nf_hook(pf, hook, net, sk, skb, in, out, okfn) == 1)
+ list_add_tail(&skb->list, &sublist);
}
+ /* Put passed packets back on main list */
+ list_splice(&sublist, head);
}
/* Call setsockopt() */
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-07-09 13:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-09 13:24 [PATCH v2 net-next 0/2] fix use-after-free bugs in skb list processing Edward Cree
2018-07-09 13:25 ` [PATCH v2 net-next 1/2] net: core: fix uses-after-free in " Edward Cree
2018-07-09 13:26 ` [PATCH v2 net-next 2/2] netfilter: fix use-after-free in NF_HOOK_LIST Edward Cree
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.