Re: [PATCH net v1] net: defer __dev_set_promiscuity() to avoid sleeping in atomic context

On 02/14, Jiayuan Chen wrote:

From: Jiayuan Chen <redacted>

__dev_set_rx_mode() is called with addr_list_lock (spinlock) held from
many places in dev_addr_lists.c. When a device lacks IFF_UNICAST_FLT,
__dev_set_rx_mode() calls __dev_set_promiscuity() which propagates
through dev_change_rx_flags -> ndo_change_rx_flags -> dev_set_promiscuity
on lower devices. Since commit 78cd408356fe ("net: add missing instance
lock to dev_set_promiscuity"), dev_set_promiscuity() acquires the netdev
instance lock (mutex) via netdev_lock_ops(). This leads to a
"sleeping function called from invalid context" / "Invalid wait context"
bug when the lower device has request_ops_lock or queue_mgmt_ops set.

The call chain is:

  dev_uc_add(bridge0)                      # e.g. from macsec_dev_open
    netif_addr_lock_bh(bridge0)            # <- spinlock, BH disabled
    __dev_set_rx_mode(bridge0)             # bridge has no IFF_UNICAST_FLT
      __dev_set_promiscuity(bridge0)
        ndo_change_rx_flags(bridge0)
          br_manage_promisc -> dev_set_promiscuity(team0)
            ndo_change_rx_flags(team0)
              team_change_rx_flags -> dev_set_promiscuity(dummy0)
                netdev_lock_ops(dummy0)    # <- mutex! dummy has
                                           #    request_ops_lock=true

This is not limited to bridge/team/dummy. Any combination of stacking
devices (bridge, bond, macvlan, vlan, macsec, team, dsa, netvsc) over
devices with instance lock (dummy, mlx5, bnxt, gve) can trigger this.

Fix this by deferring __dev_set_promiscuity() to after the spinlock is
released:

1. Change __dev_set_rx_mode() to return a promiscuity increment value
   (+1, 0, -1) instead of calling __dev_set_promiscuity() directly.
   The uc_promisc flag is still updated under the lock for correctness.

2. Change dev_set_rx_mode() to call __dev_set_promiscuity() after
   releasing addr_list_lock, based on the returned increment.

3. Change all callers in dev_addr_lists.c to release their spinlock
   first, then call dev_set_rx_mode() which handles both the rx mode
   update and the deferred promiscuity change safely.

[..]

Reproducer:

  ip link add dummy0 type dummy
  ip link add team0 type team
  ip link set dummy0 master team0
  ip link set team0 up
  ip link add bridge0 type bridge vlan_filtering 1
  ip link set bridge0 up
  ip link set team0 master bridge0
  ip link add macsec0 link bridge0 type macsec
  ip link set macsec0 up   # triggers the bug

Can you add it as a selftest under selftests/drivers/net/team/?