public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event
@ 2026-01-30 10:42 Tetsuo Handa
  2026-02-01 14:17 ` Leon Romanovsky
  2026-02-17  9:52 ` Steffen Klassert
  0 siblings, 2 replies; 6+ messages in thread
From: Tetsuo Handa @ 2026-01-30 10:42 UTC (permalink / raw)
  To: Sabrina Dubroca, Leon Romanovsky, Steffen Klassert, Herbert Xu,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Ilan Tayari, Guy Shapiro, Yossi Kuperman,
	Network Development

syzbot is reporting that "struct xfrm_state" refcount is leaking.

  unregister_netdevice: waiting for netdevsim0 to become free. Usage count = 2
  ref_tracker: netdev@ffff888052f24618 has 1/1 users at
       __netdev_tracker_alloc include/linux/netdevice.h:4400 [inline]
       netdev_tracker_alloc include/linux/netdevice.h:4412 [inline]
       xfrm_dev_state_add+0x3a5/0x1080 net/xfrm/xfrm_device.c:316
       xfrm_state_construct net/xfrm/xfrm_user.c:986 [inline]
       xfrm_add_sa+0x34ff/0x5fa0 net/xfrm/xfrm_user.c:1022
       xfrm_user_rcv_msg+0x58e/0xc00 net/xfrm/xfrm_user.c:3507
       netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2550
       xfrm_netlink_rcv+0x71/0x90 net/xfrm/xfrm_user.c:3529
       netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
       netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1344
       netlink_sendmsg+0x8c8/0xdd0 net/netlink/af_netlink.c:1894
       sock_sendmsg_nosec net/socket.c:727 [inline]
       __sock_sendmsg net/socket.c:742 [inline]
       ____sys_sendmsg+0xa5d/0xc30 net/socket.c:2592
       ___sys_sendmsg+0x134/0x1d0 net/socket.c:2646
       __sys_sendmsg+0x16d/0x220 net/socket.c:2678
       do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
       do_syscall_64+0xcd/0xf80 arch/x86/entry/syscall_64.c:94
       entry_SYSCALL_64_after_hwframe+0x77/0x7f

This is because commit d77e38e612a0 ("xfrm: Add an IPsec hardware
offloading API") implemented xfrm_dev_unregister() as no-op despite
xfrm_dev_state_add() from xfrm_state_construct() acquires a reference
to "struct net_device".
I guess that that commit expected that NETDEV_DOWN event is fired before
NETDEV_UNREGISTER event fires, and also assumed that xfrm_dev_state_add()
is called only if (dev->features & NETIF_F_HW_ESP) != 0.

Sabrina Dubroca identified steps to reproduce the same symptoms as below.

  echo 0 > /sys/bus/netdevsim/new_device
  dev=$(ls -1 /sys/bus/netdevsim/devices/netdevsim0/net/)
  ip xfrm state add src 192.168.13.1 dst 192.168.13.2 proto esp \
     spi 0x1000 mode tunnel aead 'rfc4106(gcm(aes))' $key 128   \
     offload crypto dev $dev dir out
  ethtool -K $dev esp-hw-offload off
  echo 0 > /sys/bus/netdevsim/del_device

Like these steps indicate, the NETIF_F_HW_ESP bit can be cleared after
xfrm_dev_state_add() acquired a reference to "struct net_device".
Also, xfrm_dev_state_add() does not check for the NETIF_F_HW_ESP bit
when acquiring a reference to "struct net_device".

Commit 03891f820c21 ("xfrm: handle NETDEV_UNREGISTER for xfrm device")
re-introduced the NETDEV_UNREGISTER event to xfrm_dev_event(), but that
commit for unknown reason chose to share xfrm_dev_down() between the
NETDEV_DOWN event and the NETDEV_UNREGISTER event.
I guess that that commit missed the behavior in the previous paragraph.

Therefore, we need to re-introduce xfrm_dev_unregister() in order to
release the reference to "struct net_device" by unconditionally flushing
state and policy.

Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Cc: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
v2:
  - Don't change xfrm_dev_down(), re-introduce xfrm_dev_unregister() instead.

v1:
  - https://lkml.kernel.org/r/1de734e2-1da6-4b5c-8e03-66af7f88d074@I-love.SAKURA.ne.jp

 net/xfrm/xfrm_device.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 52ae0e034d29..550457e4c4f0 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -544,6 +544,14 @@ static int xfrm_dev_down(struct net_device *dev)
 	return NOTIFY_DONE;
 }
 
+static int xfrm_dev_unregister(struct net_device *dev)
+{
+	xfrm_dev_state_flush(dev_net(dev), dev, true);
+	xfrm_dev_policy_flush(dev_net(dev), dev, true);
+
+	return NOTIFY_DONE;
+}
+
 static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
@@ -556,8 +564,10 @@ static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void
 		return xfrm_api_check(dev);
 
 	case NETDEV_DOWN:
-	case NETDEV_UNREGISTER:
 		return xfrm_dev_down(dev);
+
+	case NETDEV_UNREGISTER:
+		return xfrm_dev_unregister(dev);
 	}
 	return NOTIFY_DONE;
 }
-- 
2.47.3


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

* Re: [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event
  2026-01-30 10:42 [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event Tetsuo Handa
@ 2026-02-01 14:17 ` Leon Romanovsky
  2026-02-02 10:01   ` Steffen Klassert
  2026-02-17  9:52 ` Steffen Klassert
  1 sibling, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2026-02-01 14:17 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Sabrina Dubroca, Steffen Klassert, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Ilan Tayari, Guy Shapiro, Yossi Kuperman, Network Development

On Fri, Jan 30, 2026 at 07:42:47PM +0900, Tetsuo Handa wrote:
> syzbot is reporting that "struct xfrm_state" refcount is leaking.
> 
>   unregister_netdevice: waiting for netdevsim0 to become free. Usage count = 2
>   ref_tracker: netdev@ffff888052f24618 has 1/1 users at
>        __netdev_tracker_alloc include/linux/netdevice.h:4400 [inline]
>        netdev_tracker_alloc include/linux/netdevice.h:4412 [inline]
>        xfrm_dev_state_add+0x3a5/0x1080 net/xfrm/xfrm_device.c:316
>        xfrm_state_construct net/xfrm/xfrm_user.c:986 [inline]
>        xfrm_add_sa+0x34ff/0x5fa0 net/xfrm/xfrm_user.c:1022
>        xfrm_user_rcv_msg+0x58e/0xc00 net/xfrm/xfrm_user.c:3507
>        netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2550
>        xfrm_netlink_rcv+0x71/0x90 net/xfrm/xfrm_user.c:3529
>        netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
>        netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1344
>        netlink_sendmsg+0x8c8/0xdd0 net/netlink/af_netlink.c:1894
>        sock_sendmsg_nosec net/socket.c:727 [inline]
>        __sock_sendmsg net/socket.c:742 [inline]
>        ____sys_sendmsg+0xa5d/0xc30 net/socket.c:2592
>        ___sys_sendmsg+0x134/0x1d0 net/socket.c:2646
>        __sys_sendmsg+0x16d/0x220 net/socket.c:2678
>        do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>        do_syscall_64+0xcd/0xf80 arch/x86/entry/syscall_64.c:94
>        entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> This is because commit d77e38e612a0 ("xfrm: Add an IPsec hardware
> offloading API") implemented xfrm_dev_unregister() as no-op despite
> xfrm_dev_state_add() from xfrm_state_construct() acquires a reference
> to "struct net_device".
> I guess that that commit expected that NETDEV_DOWN event is fired before
> NETDEV_UNREGISTER event fires, and also assumed that xfrm_dev_state_add()
> is called only if (dev->features & NETIF_F_HW_ESP) != 0.
> 
> Sabrina Dubroca identified steps to reproduce the same symptoms as below.
> 
>   echo 0 > /sys/bus/netdevsim/new_device
>   dev=$(ls -1 /sys/bus/netdevsim/devices/netdevsim0/net/)
>   ip xfrm state add src 192.168.13.1 dst 192.168.13.2 proto esp \
>      spi 0x1000 mode tunnel aead 'rfc4106(gcm(aes))' $key 128   \
>      offload crypto dev $dev dir out
>   ethtool -K $dev esp-hw-offload off
>   echo 0 > /sys/bus/netdevsim/del_device
> 
> Like these steps indicate, the NETIF_F_HW_ESP bit can be cleared after
> xfrm_dev_state_add() acquired a reference to "struct net_device".
> Also, xfrm_dev_state_add() does not check for the NETIF_F_HW_ESP bit
> when acquiring a reference to "struct net_device".
> 
> Commit 03891f820c21 ("xfrm: handle NETDEV_UNREGISTER for xfrm device")
> re-introduced the NETDEV_UNREGISTER event to xfrm_dev_event(), but that
> commit for unknown reason chose to share xfrm_dev_down() between the
> NETDEV_DOWN event and the NETDEV_UNREGISTER event.
> I guess that that commit missed the behavior in the previous paragraph.
> 
> Therefore, we need to re-introduce xfrm_dev_unregister() in order to
> release the reference to "struct net_device" by unconditionally flushing
> state and policy.
> 
> Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
> Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
> Cc: Sabrina Dubroca <sd@queasysnail.net>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> v2:
>   - Don't change xfrm_dev_down(), re-introduce xfrm_dev_unregister() instead.
> 
> v1:
>   - https://lkml.kernel.org/r/1de734e2-1da6-4b5c-8e03-66af7f88d074@I-love.SAKURA.ne.jp
> 
>  net/xfrm/xfrm_device.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)

I believe this is the correct solution, but I do not feel confident enough to
add my Reviewed-by.

Steffen,

If you decide to take it, please queue it in your -next branch, so we will have enough
time to test in our regression. It seems too risky for -rc8.

Thanks

> 
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 52ae0e034d29..550457e4c4f0 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -544,6 +544,14 @@ static int xfrm_dev_down(struct net_device *dev)
>  	return NOTIFY_DONE;
>  }
>  
> +static int xfrm_dev_unregister(struct net_device *dev)
> +{
> +	xfrm_dev_state_flush(dev_net(dev), dev, true);
> +	xfrm_dev_policy_flush(dev_net(dev), dev, true);
> +
> +	return NOTIFY_DONE;
> +}
> +
>  static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
>  {
>  	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> @@ -556,8 +564,10 @@ static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void
>  		return xfrm_api_check(dev);
>  
>  	case NETDEV_DOWN:
> -	case NETDEV_UNREGISTER:
>  		return xfrm_dev_down(dev);
> +
> +	case NETDEV_UNREGISTER:
> +		return xfrm_dev_unregister(dev);
>  	}
>  	return NOTIFY_DONE;
>  }
> -- 
> 2.47.3
> 
> 

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

* Re: [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event
  2026-02-01 14:17 ` Leon Romanovsky
@ 2026-02-02 10:01   ` Steffen Klassert
  2026-02-02 12:36     ` Leon Romanovsky
  0 siblings, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2026-02-02 10:01 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Tetsuo Handa, Sabrina Dubroca, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Ilan Tayari, Guy Shapiro, Yossi Kuperman, Network Development

On Sun, Feb 01, 2026 at 04:17:39PM +0200, Leon Romanovsky wrote:
> 
> Steffen,
> 
> If you decide to take it, please queue it in your -next branch, so we will have enough
> time to test in our regression. It seems too risky for -rc8.

I plan to take the patch as is. It is a fix, so I'll
take it into the ipsec tree, not ipsec-next. But
I can hold it off until after the release if that
will help regression testing.

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

* Re: [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event
  2026-02-02 10:01   ` Steffen Klassert
@ 2026-02-02 12:36     ` Leon Romanovsky
  0 siblings, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2026-02-02 12:36 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Tetsuo Handa, Sabrina Dubroca, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Ilan Tayari, Guy Shapiro, Yossi Kuperman, Network Development,
	Tariq Toukan

On Mon, Feb 02, 2026 at 11:01:28AM +0100, Steffen Klassert wrote:
> On Sun, Feb 01, 2026 at 04:17:39PM +0200, Leon Romanovsky wrote:
> > 
> > Steffen,
> > 
> > If you decide to take it, please queue it in your -next branch, so we will have enough
> > time to test in our regression. It seems too risky for -rc8.
> 
> I plan to take the patch as is. It is a fix, so I'll
> take it into the ipsec tree, not ipsec-next. But
> I can hold it off until after the release if that
> will help regression testing.

Let's wait until the end of the week. We will include this fix in tonight's
nightly regression, and I will need a couple of runs to reduce the risk of a
statistical error.

Thanks

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

* Re: [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event
  2026-01-30 10:42 [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event Tetsuo Handa
  2026-02-01 14:17 ` Leon Romanovsky
@ 2026-02-17  9:52 ` Steffen Klassert
  2026-02-17 13:45   ` Leon Romanovsky
  1 sibling, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2026-02-17  9:52 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Sabrina Dubroca, Leon Romanovsky, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Ilan Tayari, Guy Shapiro, Yossi Kuperman, Network Development

On Fri, Jan 30, 2026 at 07:42:47PM +0900, Tetsuo Handa wrote:
> syzbot is reporting that "struct xfrm_state" refcount is leaking.
> 
>   unregister_netdevice: waiting for netdevsim0 to become free. Usage count = 2
>   ref_tracker: netdev@ffff888052f24618 has 1/1 users at
>        __netdev_tracker_alloc include/linux/netdevice.h:4400 [inline]
>        netdev_tracker_alloc include/linux/netdevice.h:4412 [inline]
>        xfrm_dev_state_add+0x3a5/0x1080 net/xfrm/xfrm_device.c:316
>        xfrm_state_construct net/xfrm/xfrm_user.c:986 [inline]
>        xfrm_add_sa+0x34ff/0x5fa0 net/xfrm/xfrm_user.c:1022
>        xfrm_user_rcv_msg+0x58e/0xc00 net/xfrm/xfrm_user.c:3507
>        netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2550
>        xfrm_netlink_rcv+0x71/0x90 net/xfrm/xfrm_user.c:3529
>        netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
>        netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1344
>        netlink_sendmsg+0x8c8/0xdd0 net/netlink/af_netlink.c:1894
>        sock_sendmsg_nosec net/socket.c:727 [inline]
>        __sock_sendmsg net/socket.c:742 [inline]
>        ____sys_sendmsg+0xa5d/0xc30 net/socket.c:2592
>        ___sys_sendmsg+0x134/0x1d0 net/socket.c:2646
>        __sys_sendmsg+0x16d/0x220 net/socket.c:2678
>        do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>        do_syscall_64+0xcd/0xf80 arch/x86/entry/syscall_64.c:94
>        entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> This is because commit d77e38e612a0 ("xfrm: Add an IPsec hardware
> offloading API") implemented xfrm_dev_unregister() as no-op despite
> xfrm_dev_state_add() from xfrm_state_construct() acquires a reference
> to "struct net_device".
> I guess that that commit expected that NETDEV_DOWN event is fired before
> NETDEV_UNREGISTER event fires, and also assumed that xfrm_dev_state_add()
> is called only if (dev->features & NETIF_F_HW_ESP) != 0.
> 
> Sabrina Dubroca identified steps to reproduce the same symptoms as below.
> 
>   echo 0 > /sys/bus/netdevsim/new_device
>   dev=$(ls -1 /sys/bus/netdevsim/devices/netdevsim0/net/)
>   ip xfrm state add src 192.168.13.1 dst 192.168.13.2 proto esp \
>      spi 0x1000 mode tunnel aead 'rfc4106(gcm(aes))' $key 128   \
>      offload crypto dev $dev dir out
>   ethtool -K $dev esp-hw-offload off
>   echo 0 > /sys/bus/netdevsim/del_device
> 
> Like these steps indicate, the NETIF_F_HW_ESP bit can be cleared after
> xfrm_dev_state_add() acquired a reference to "struct net_device".
> Also, xfrm_dev_state_add() does not check for the NETIF_F_HW_ESP bit
> when acquiring a reference to "struct net_device".
> 
> Commit 03891f820c21 ("xfrm: handle NETDEV_UNREGISTER for xfrm device")
> re-introduced the NETDEV_UNREGISTER event to xfrm_dev_event(), but that
> commit for unknown reason chose to share xfrm_dev_down() between the
> NETDEV_DOWN event and the NETDEV_UNREGISTER event.
> I guess that that commit missed the behavior in the previous paragraph.
> 
> Therefore, we need to re-introduce xfrm_dev_unregister() in order to
> release the reference to "struct net_device" by unconditionally flushing
> state and policy.
> 
> Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
> Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
> Cc: Sabrina Dubroca <sd@queasysnail.net>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Now applied to the ipsec tree, thanks a lot!

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

* Re: [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event
  2026-02-17  9:52 ` Steffen Klassert
@ 2026-02-17 13:45   ` Leon Romanovsky
  0 siblings, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2026-02-17 13:45 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Tetsuo Handa, Sabrina Dubroca, Herbert Xu, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Ilan Tayari, Guy Shapiro, Yossi Kuperman, Network Development

On Tue, Feb 17, 2026 at 10:52:43AM +0100, Steffen Klassert wrote:
> On Fri, Jan 30, 2026 at 07:42:47PM +0900, Tetsuo Handa wrote:
> > syzbot is reporting that "struct xfrm_state" refcount is leaking.
> > 
> >   unregister_netdevice: waiting for netdevsim0 to become free. Usage count = 2
> >   ref_tracker: netdev@ffff888052f24618 has 1/1 users at
> >        __netdev_tracker_alloc include/linux/netdevice.h:4400 [inline]
> >        netdev_tracker_alloc include/linux/netdevice.h:4412 [inline]
> >        xfrm_dev_state_add+0x3a5/0x1080 net/xfrm/xfrm_device.c:316
> >        xfrm_state_construct net/xfrm/xfrm_user.c:986 [inline]
> >        xfrm_add_sa+0x34ff/0x5fa0 net/xfrm/xfrm_user.c:1022
> >        xfrm_user_rcv_msg+0x58e/0xc00 net/xfrm/xfrm_user.c:3507
> >        netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2550
> >        xfrm_netlink_rcv+0x71/0x90 net/xfrm/xfrm_user.c:3529
> >        netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
> >        netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1344
> >        netlink_sendmsg+0x8c8/0xdd0 net/netlink/af_netlink.c:1894
> >        sock_sendmsg_nosec net/socket.c:727 [inline]
> >        __sock_sendmsg net/socket.c:742 [inline]
> >        ____sys_sendmsg+0xa5d/0xc30 net/socket.c:2592
> >        ___sys_sendmsg+0x134/0x1d0 net/socket.c:2646
> >        __sys_sendmsg+0x16d/0x220 net/socket.c:2678
> >        do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> >        do_syscall_64+0xcd/0xf80 arch/x86/entry/syscall_64.c:94
> >        entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > 
> > This is because commit d77e38e612a0 ("xfrm: Add an IPsec hardware
> > offloading API") implemented xfrm_dev_unregister() as no-op despite
> > xfrm_dev_state_add() from xfrm_state_construct() acquires a reference
> > to "struct net_device".
> > I guess that that commit expected that NETDEV_DOWN event is fired before
> > NETDEV_UNREGISTER event fires, and also assumed that xfrm_dev_state_add()
> > is called only if (dev->features & NETIF_F_HW_ESP) != 0.
> > 
> > Sabrina Dubroca identified steps to reproduce the same symptoms as below.
> > 
> >   echo 0 > /sys/bus/netdevsim/new_device
> >   dev=$(ls -1 /sys/bus/netdevsim/devices/netdevsim0/net/)
> >   ip xfrm state add src 192.168.13.1 dst 192.168.13.2 proto esp \
> >      spi 0x1000 mode tunnel aead 'rfc4106(gcm(aes))' $key 128   \
> >      offload crypto dev $dev dir out
> >   ethtool -K $dev esp-hw-offload off
> >   echo 0 > /sys/bus/netdevsim/del_device
> > 
> > Like these steps indicate, the NETIF_F_HW_ESP bit can be cleared after
> > xfrm_dev_state_add() acquired a reference to "struct net_device".
> > Also, xfrm_dev_state_add() does not check for the NETIF_F_HW_ESP bit
> > when acquiring a reference to "struct net_device".
> > 
> > Commit 03891f820c21 ("xfrm: handle NETDEV_UNREGISTER for xfrm device")
> > re-introduced the NETDEV_UNREGISTER event to xfrm_dev_event(), but that
> > commit for unknown reason chose to share xfrm_dev_down() between the
> > NETDEV_DOWN event and the NETDEV_UNREGISTER event.
> > I guess that that commit missed the behavior in the previous paragraph.
> > 
> > Therefore, we need to re-introduce xfrm_dev_unregister() in order to
> > release the reference to "struct net_device" by unconditionally flushing
> > state and policy.
> > 
> > Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
> > Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
> > Cc: Sabrina Dubroca <sd@queasysnail.net>
> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> 
> Now applied to the ipsec tree, thanks a lot!

Thanks, I also didn't hear any bad news from our regression too.

Thanks

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

end of thread, other threads:[~2026-02-17 13:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-30 10:42 [PATCH net v2] xfrm: always flush state and policy upon NETDEV_UNREGISTER event Tetsuo Handa
2026-02-01 14:17 ` Leon Romanovsky
2026-02-02 10:01   ` Steffen Klassert
2026-02-02 12:36     ` Leon Romanovsky
2026-02-17  9:52 ` Steffen Klassert
2026-02-17 13:45   ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox