[PATCH] net: hsr: avoid synchronize_net() in hsr_del_port() under rtnl_mutex

[PATCH] net: hsr: avoid synchronize_net() in hsr_del_port() under rtnl_mutex

[Shardul Bankar]

hsr_del_port() calls netdev_rx_handler_unregister(), which calls
synchronize_net() while rtnl_mutex is held.  During netns teardown,
cleanup_net() walks pernet_list and reaches default_device_exit_batch(),
which takes rtnl_mutex and iterates devices calling
rtnl_link_ops->dellink() for each.  For HSR this resolves to
hsr_dellink() -> hsr_del_ports() -> hsr_del_port(), so the
synchronize_net() call runs under rtnl_mutex.  Under contention this
stalls other rtnl_mutex waiters long enough to trip the hung-task
detector:

  kworker cleanup_net -> default_device_exit_batch -> hsr_del_port
    -> netdev_rx_handler_unregister -> synchronize_rcu_expedited
    [slow, holds rtnl_mutex]
  syz-executor -> ksys_unshare -> setup_net -> ip_tunnel_init_net
    -> rtnl_lock  [blocked on rtnl_mutex]

Open-code netdev_rx_handler_unregister() in hsr_del_port() without
the synchronize_net() step.  This is safe because hsr_del_port()
already defers the port free via kfree_rcu(port, rcu), so the
rx_handler_data memory remains valid until an RCU grace period
elapses naturally.  hsr_handle_frame() additionally re-validates
rx_handler via hsr_port_get_rcu() before dereferencing
rx_handler_data, so a reader that observes rx_handler_data == NULL
in the brief window between the two clears takes the NULL path and
returns RX_HANDLER_PASS without touching the port.

Reported-by: syzbot+f2fbf7478a35a94c8b7c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?id=cb64c22a492202ca929e18262fdb8cb89e635c70
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
Testing status:

The hang was originally observed on 7.0-rc6 (commit 7ca6d1cfec80) with
the syzkaller reproducer and a KASAN+LOCKDEP config.  Subsequent
reproduction attempts with the same reproducer on current mainline did
not retrigger this specific signature, so I could not verify the fix
against a live reproducer.  The patch is submitted on the basis of
code analysis.

 net/hsr/hsr_slave.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index d9af9e65f72f..5e92f23fa1a5 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -239,7 +239,18 @@ void hsr_del_port(struct hsr_port *port)
 	if (port != master) {
 		netdev_update_features(master->dev);
 		dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
-		netdev_rx_handler_unregister(port->dev);
+		/* Open-code netdev_rx_handler_unregister() without the
+		 * synchronize_net() step: holding rtnl_mutex across the
+		 * grace-period wait stalls other rtnl_mutex waiters long
+		 * enough to trip the hung-task detector under load.
+		 * Skipping synchronize_net() is safe because the port is
+		 * freed via kfree_rcu() below (so rx_handler_data memory
+		 * outlives any in-flight reader), and hsr_handle_frame()
+		 * re-validates rx_handler via hsr_port_get_rcu() before
+		 * dereferencing rx_handler_data.
+		 */
+		RCU_INIT_POINTER(port->dev->rx_handler, NULL);
+		RCU_INIT_POINTER(port->dev->rx_handler_data, NULL);
 		if (!port->hsr->fwd_offloaded)
 			dev_set_promiscuity(port->dev, -1);
 		netdev_upper_dev_unlink(port->dev, master->dev);
-- 
2.34.1

Re: [PATCH] net: hsr: avoid synchronize_net() in hsr_del_port() under rtnl_mutex

[Eric Dumazet]

On Fri, Apr 17, 2026 at 10:53 AM Shardul Bankar <shardulsb08@gmail.com> wrote:
>
> hsr_del_port() calls netdev_rx_handler_unregister(), which calls
> synchronize_net() while rtnl_mutex is held.  During netns teardown,
> cleanup_net() walks pernet_list and reaches default_device_exit_batch(),
> which takes rtnl_mutex and iterates devices calling
> rtnl_link_ops->dellink() for each.  For HSR this resolves to
> hsr_dellink() -> hsr_del_ports() -> hsr_del_port(), so the
> synchronize_net() call runs under rtnl_mutex.  Under contention this
> stalls other rtnl_mutex waiters long enough to trip the hung-task
> detector:
>
>   kworker cleanup_net -> default_device_exit_batch -> hsr_del_port
>     -> netdev_rx_handler_unregister -> synchronize_rcu_expedited
>     [slow, holds rtnl_mutex]

 synchronize_rcu_expedited() should be quite fast...



>   syz-executor -> ksys_unshare -> setup_net -> ip_tunnel_init_net
>     -> rtnl_lock  [blocked on rtnl_mutex]
>
> Open-code netdev_rx_handler_unregister() in hsr_del_port() without
> the synchronize_net() step.  This is safe because hsr_del_port()
> already defers the port free via kfree_rcu(port, rcu), so the
> rx_handler_data memory remains valid until an RCU grace period
> elapses naturally.  hsr_handle_frame() additionally re-validates
> rx_handler via hsr_port_get_rcu() before dereferencing
> rx_handler_data, so a reader that observes rx_handler_data == NULL
> in the brief window between the two clears takes the NULL path and
> returns RX_HANDLER_PASS without touching the port.
>
> Reported-by: syzbot+f2fbf7478a35a94c8b7c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=cb64c22a492202ca929e18262fdb8cb89e635c70

Signature looks like bug fixed recently in wireguard.

commit 60a25ef8dacb3566b1a8c4de00572a498e2a3bf9
Author: Shardul Bankar <shardul.b@mpiricsoftware.com>
Date:   Tue Apr 14 17:39:44 2026 +0200

    wireguard: device: use exit_rtnl callback instead of manual
rtnl_lock in pre_exit



> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
> Testing status:
>
> The hang was originally observed on 7.0-rc6 (commit 7ca6d1cfec80) with
> the syzkaller reproducer and a KASAN+LOCKDEP config.  Subsequent
> reproduction attempts with the same reproducer on current mainline did
> not retrigger this specific signature, so I could not verify the fix
> against a live reproducer.  The patch is submitted on the basis of
> code analysis.
>
>  net/hsr/hsr_slave.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index d9af9e65f72f..5e92f23fa1a5 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -239,7 +239,18 @@ void hsr_del_port(struct hsr_port *port)
>         if (port != master) {
>                 netdev_update_features(master->dev);
>                 dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
> -               netdev_rx_handler_unregister(port->dev);
> +               /* Open-code netdev_rx_handler_unregister() without the
> +                * synchronize_net() step: holding rtnl_mutex across the
> +                * grace-period wait stalls other rtnl_mutex waiters long
> +                * enough to trip the hung-task detector under load.
> +                * Skipping synchronize_net() is safe because the port is
> +                * freed via kfree_rcu() below (so rx_handler_data memory
> +                * outlives any in-flight reader), and hsr_handle_frame()
> +                * re-validates rx_handler via hsr_port_get_rcu() before
> +                * dereferencing rx_handler_data.
> +                */
> +               RCU_INIT_POINTER(port->dev->rx_handler, NULL);
> +               RCU_INIT_POINTER(port->dev->rx_handler_data, NULL);
>                 if (!port->hsr->fwd_offloaded)
>                         dev_set_promiscuity(port->dev, -1);
>                 netdev_upper_dev_unlink(port->dev, master->dev);

You are saying that all netdev_rx_handler_unregister() uses are
potentially a problem.

This can not be true.

Re: [PATCH] net: hsr: avoid synchronize_net() in hsr_del_port() under rtnl_mutex

[Shardul Bankar]

On Fri, 2026-04-17 at 11:18 -0700, Eric Dumazet wrote:
> On Fri, Apr 17, 2026 at 10:53 AM Shardul Bankar
> <shardulsb08@gmail.com> wrote:
> > 
> 
>  synchronize_rcu_expedited() should be quite fast...

Confirmed.  I ran a targeted HSR-only teardown stress on current
mainline (many HSR-bearing netns torn down concurrently, no WireGuard,
no syzkaller harness).  At N=1000, patched-vs-baseline cleanup_net
wall time is within run-to-run noise- the patch has no measurable
benefit on the workload it was meant to address.


> Signature looks like bug fixed recently in wireguard.
> 
> commit 60a25ef8dacb3566b1a8c4de00572a498e2a3bf9

Agreed.  60a25ef8dacb covers what I was actually observing, and
standalone evidence for the HSR path on current mainline is not
strong enough to justify this change.

Please drop this patch.

Thanks,
Shardul