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@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?id=cb64c22a492202ca929e18262fdb8cb89e635c70

Signature looks like bug fixed recently in wireguard.

commit 60a25ef8dacb3566b1a8c4de00572a498e2a3bf9
Author: Shardul Bankar <shardul.b@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxx>
> ---
> 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.