All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] xfrm: add missing rcu_read_lock(), skb_dst_force() and dev_hold() for xfrm_trans_reinject()
@ 2026-08-07 17:15 Eric Dumazet
  2026-08-17  8:16 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-08-07 17:15 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, netdev, eric.dumazet, Eric Dumazet, syzbot,
	Steffen Klassert, Liu Jian

syzbot reported a suspicious RCU usage warning in ip6_pkt_drop():

  WARNING: suspicious RCU usage in ip6_pkt_drop
  include/net/addrconf.h:389 suspicious rcu_dereference_check() usage!

  Call Trace:
   __in6_dev_get_safely include/net/addrconf.h:389 [inline]
   ip6_pkt_drop+0x596/0x610 net/ipv6/route.c:4620
   ip6_pkt_discard+0x1c/0x30 net/ipv6/route.c:4651
   xfrm_trans_reinject+0x324/0x630 net/xfrm/xfrm_input.c:806
   process_one_work kernel/workqueue.c:3322 [inline]
   process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
   worker_thread+0xa47/0xfb0 kernel/workqueue.c:3486

When commit 4f4920669d21 ("xfrm: Reinject transport-mode packets through
workqueue") converted xfrm_trans_reinject from a tasklet to a workqueue,
the reinjection loop ceased running in softirq context. Workqueue workers
run in process context where local_bh_disable() does not enter an RCU
read-side critical section under CONFIG_PREEMPT_RCU.

Because finish callbacks (such as ip6_rcv_finish) expect to run under an
RCU read lock (performing route lookups, l3mdev lookups, and accessing
RCU-protected data structures), invoking them in workqueue context without
rcu_read_lock() triggers RCU lockdep warnings.

Furthermore, packets queued to the workqueue via xfrm_trans_queue_net()
may carry non-refcounted (noref) dst entries (e.g. from ip_route_input_noref).
Additionally, on netdevice unregistration, dst_dev_put() replaces dst->dev
with blackhole_netdev, so dst entries do not keep skb->dev alive while
queued in the workqueue.

Fix these issues by:
1. Calling skb_dst_force(skb) in xfrm_trans_queue_net() while still in the
   caller's RCU section to ensure dst is reference-counted before queuing.
2. Holding a reference on skb->dev via dev_hold()/dev_put() across workqueue
   deferral so skb->dev remains valid during finish() callback processing.
3. Acquiring rcu_read_lock() around the finish callback invocation loop in
   xfrm_trans_reinject().

Fixes: 4f4920669d21 ("xfrm: Reinject transport-mode packets through workqueue")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Liu Jian <liujian56@huawei.com>
---
 net/xfrm/xfrm_input.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index eecab337bd0a794588b192598851bd77427c8392..8f6109eada7eaaf1c70aa2da610523b42d75b50c 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -800,12 +800,17 @@ static void xfrm_trans_reinject(struct work_struct *work)
 	spin_unlock_bh(&trans->queue_lock);
 
 	local_bh_disable();
+	rcu_read_lock();
 	while ((skb = __skb_dequeue(&queue))) {
 		struct net *net = XFRM_TRANS_SKB_CB(skb)->net;
+		struct net_device *dev = skb->dev;
 
 		XFRM_TRANS_SKB_CB(skb)->finish(net, NULL, skb);
+		if (dev)
+			dev_put(dev);
 		put_net(net);
 	}
+	rcu_read_unlock();
 	local_bh_enable();
 }
 
@@ -821,12 +826,18 @@ int xfrm_trans_queue_net(struct net *net, struct sk_buff *skb,
 	if (skb_queue_len(&trans->queue) >= READ_ONCE(net_hotdata.max_backlog))
 		return -ENOBUFS;
 
+	if (skb_dst(skb) && !skb_dst_force(skb))
+		return -EHOSTUNREACH;
+
 	BUILD_BUG_ON(sizeof(struct xfrm_trans_cb) > sizeof(skb->cb));
 
 	hold_net = maybe_get_net(net);
 	if (!hold_net)
 		return -ENODEV;
 
+	if (skb->dev)
+		dev_hold(skb->dev);
+
 	XFRM_TRANS_SKB_CB(skb)->finish = finish;
 	XFRM_TRANS_SKB_CB(skb)->net = hold_net;
 	spin_lock_bh(&trans->queue_lock);
-- 
2.55.0.654.g21b8a5bc05-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] xfrm: add missing rcu_read_lock(), skb_dst_force() and dev_hold() for xfrm_trans_reinject()
  2026-08-07 17:15 [PATCH net] xfrm: add missing rcu_read_lock(), skb_dst_force() and dev_hold() for xfrm_trans_reinject() Eric Dumazet
@ 2026-08-17  8:16 ` Eric Dumazet
  2026-08-17  8:20   ` Steffen Klassert
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-08-17  8:16 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, netdev, eric.dumazet, syzbot, Steffen Klassert,
	Liu Jian

On Fri, Aug 7, 2026 at 7:15 PM Eric Dumazet <edumazet@google.com> wrote:
>
> syzbot reported a suspicious RCU usage warning in ip6_pkt_drop():
>
>   WARNING: suspicious RCU usage in ip6_pkt_drop
>   include/net/addrconf.h:389 suspicious rcu_dereference_check() usage!
>
>   Call Trace:
>    __in6_dev_get_safely include/net/addrconf.h:389 [inline]
>    ip6_pkt_drop+0x596/0x610 net/ipv6/route.c:4620
>    ip6_pkt_discard+0x1c/0x30 net/ipv6/route.c:4651
>    xfrm_trans_reinject+0x324/0x630 net/xfrm/xfrm_input.c:806
>    process_one_work kernel/workqueue.c:3322 [inline]
>    process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
>    worker_thread+0xa47/0xfb0 kernel/workqueue.c:3486
>
> When commit 4f4920669d21 ("xfrm: Reinject transport-mode packets through
> workqueue") converted xfrm_trans_reinject from a tasklet to a workqueue,
> the reinjection loop ceased running in softirq context. Workqueue workers
> run in process context where local_bh_disable() does not enter an RCU
> read-side critical section under CONFIG_PREEMPT_RCU.
>
> Because finish callbacks (such as ip6_rcv_finish) expect to run under an
> RCU read lock (performing route lookups, l3mdev lookups, and accessing
> RCU-protected data structures), invoking them in workqueue context without
> rcu_read_lock() triggers RCU lockdep warnings.
>
> Furthermore, packets queued to the workqueue via xfrm_trans_queue_net()
> may carry non-refcounted (noref) dst entries (e.g. from ip_route_input_noref).
> Additionally, on netdevice unregistration, dst_dev_put() replaces dst->dev
> with blackhole_netdev, so dst entries do not keep skb->dev alive while
> queued in the workqueue.
>
> Fix these issues by:
> 1. Calling skb_dst_force(skb) in xfrm_trans_queue_net() while still in the
>    caller's RCU section to ensure dst is reference-counted before queuing.
> 2. Holding a reference on skb->dev via dev_hold()/dev_put() across workqueue
>    deferral so skb->dev remains valid during finish() callback processing.
> 3. Acquiring rcu_read_lock() around the finish callback invocation loop in
>    xfrm_trans_reinject().
>
> Fixes: 4f4920669d21 ("xfrm: Reinject transport-mode packets through workqueue")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Cc: Liu Jian <liujian56@huawei.com>
> ---
>  net/xfrm/xfrm_input.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index eecab337bd0a794588b192598851bd77427c8392..8f6109eada7eaaf1c70aa2da610523b42d75b50c 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -800,12 +800,17 @@ static void xfrm_trans_reinject(struct work_struct *work)
>         spin_unlock_bh(&trans->queue_lock);
>
>         local_bh_disable();
> +       rcu_read_lock();
>         while ((skb = __skb_dequeue(&queue))) {
>                 struct net *net = XFRM_TRANS_SKB_CB(skb)->net;
> +               struct net_device *dev = skb->dev;
>
>                 XFRM_TRANS_SKB_CB(skb)->finish(net, NULL, skb);
> +               if (dev)
> +                       dev_put(dev);
>                 put_net(net);
>         }
> +       rcu_read_unlock();
>         local_bh_enable();
>  }
>
> @@ -821,12 +826,18 @@ int xfrm_trans_queue_net(struct net *net, struct sk_buff *skb,
>         if (skb_queue_len(&trans->queue) >= READ_ONCE(net_hotdata.max_backlog))
>                 return -ENOBUFS;
>
> +       if (skb_dst(skb) && !skb_dst_force(skb))
> +               return -EHOSTUNREACH;
> +
>         BUILD_BUG_ON(sizeof(struct xfrm_trans_cb) > sizeof(skb->cb));
>
>         hold_net = maybe_get_net(net);
>         if (!hold_net)
>                 return -ENODEV;
>
> +       if (skb->dev)
> +               dev_hold(skb->dev);
> +
>         XFRM_TRANS_SKB_CB(skb)->finish = finish;
>         XFRM_TRANS_SKB_CB(skb)->net = hold_net;
>         spin_lock_bh(&trans->queue_lock);
> --
> 2.55.0.654.g21b8a5bc05-goog
>

Gentle ping, thanks !

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] xfrm: add missing rcu_read_lock(), skb_dst_force() and dev_hold() for xfrm_trans_reinject()
  2026-08-17  8:16 ` Eric Dumazet
@ 2026-08-17  8:20   ` Steffen Klassert
  0 siblings, 0 replies; 3+ messages in thread
From: Steffen Klassert @ 2026-08-17  8:20 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	netdev, eric.dumazet, syzbot, Liu Jian

On Mon, Aug 17, 2026 at 10:16:35AM +0200, Eric Dumazet wrote:
> On Fri, Aug 7, 2026 at 7:15 PM Eric Dumazet <edumazet@google.com> wrote:
> 
> Gentle ping, thanks !

Will look at it today, thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-17  8:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07 17:15 [PATCH net] xfrm: add missing rcu_read_lock(), skb_dst_force() and dev_hold() for xfrm_trans_reinject() Eric Dumazet
2026-08-17  8:16 ` Eric Dumazet
2026-08-17  8:20   ` Steffen Klassert

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.