kernel/linux
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ synced 2026-11-10 20:48:12 +00:00
Code Issues Releases Packages Activity

net: use two lockdep classes for the netdev instance lock

Browse source 
netdev_lockdep_set_classes() puts dev->lock in a separate lockdep class,
by type (netkit vs dummy etc), with the intent of keeping the instance
locks of individual devices as independent from each other as possible.
In practice it does the opposite. lockdep only calls the cmp_fn for locks
of the same class, so netdev_lock_cmp_fn() never gets a say when devices
from different classes are nested. Instead lockdep records a dependency
between the classes, and reports a circular locking problem as soon as
the nesting happens the other way round, e.g. when devices are unregistered
in a batch.

  ======================================================
  WARNING: possible circular locking dependency detected
  7.3.0-rc3+ #26 Not tainted
  ------------------------------------------------------
  kworker/u256:1/326 is trying to acquire lock:
  ff110000104fce28 (&dev_instance_lock_key#6){+.+.}-{4:4}, at:
  unregister_netdevice_many_notify+0x1141/0x1c30

  but task is already holding lock:
  ff110000127f2e28 (&dev_instance_lock_key#7){+.+.}-{4:4}, at:
  unregister_netdevice_many_notify+0x1141/0x1c30

  -> #1 (&dev_instance_lock_key#7){+.+.}-{4:4}:
         __lock_acquire+0x767/0xd60
         lock_acquire.part.0+0xd0/0x260
         __mutex_lock+0x17d/0x1f20
         unregister_netdevice_many_notify+0x1141/0x1c30
         default_device_exit_batch+0x3ee/0x520
         ops_undo_list+0x2cc/0x8a0
         cleanup_net+0x442/0x9c0
         process_one_work+0x951/0x1ab0
         worker_thread+0x5a6/0xd10
         kthread+0x339/0x430
         ret_from_fork+0x4a4/0x6f0
         ret_from_fork_asm+0x1a/0x30

  -> #0 (&dev_instance_lock_key#6){+.+.}-{4:4}:
         check_prev_add+0xeb/0xe60
         validate_chain+0x598/0x900
         __lock_acquire+0x767/0xd60
         lock_acquire.part.0+0xd0/0x260
         __mutex_lock+0x17d/0x1f20
         unregister_netdevice_many_notify+0x1141/0x1c30
         default_device_exit_batch+0x3ee/0x520
         ops_undo_list+0x2cc/0x8a0
         cleanup_net+0x442/0x9c0
         process_one_work+0x951/0x1ab0
         worker_thread+0x5a6/0xd10
         kthread+0x339/0x430
         ret_from_fork+0x4a4/0x6f0
         ret_from_fork_asm+0x1a/0x30

   Possible unsafe locking scenario:

         CPU0                    CPU1
         ----                    ----
    lock(&dev_instance_lock_key#7);
                                 lock(&dev_instance_lock_key#6);
                                 lock(&dev_instance_lock_key#7);
    lock(&dev_instance_lock_key#6);

   *** DEADLOCK ***

  locks held by kworker/u256:1/326: 6, last CPU#6:
   #0: ff11000001c2b540 ((wq_completion)netns){+.+.}-{0:0}, at:
  process_one_work+0x117c/0x1ab0
   #1: ffa0000001a3fd18 (net_cleanup_work){+.+.}-{0:0}, at:
  process_one_work+0x8ce/0x1ab0
   #2: ffffffff98f53288 (pernet_ops_rwsem){++++}-{4:4}, at:
  cleanup_net+0xc1/0x9c0
   #3: ffffffff98f6ede0 (rtnl_mutex){+.+.}-{4:4}, at:
  default_device_exit_batch+0x92/0x520
   #4: ff1100001321ae28 (&dev_instance_lock_key#7){+.+.}-{4:4}, at:
  unregister_netdevice_many_notify+0x1141/0x1c30

We can't put all devices in the same class, that'd be too permissive.
netdev_nl_queue_create_doit() and netdev_nl_bind_tx_doit() lock
a virtual device (netkit) before a physical device, without rtnl_lock.
We can never allow locking in the opposite order even under rtnl_lock.
cmp_fn cannot enforce this sort of rule: lockdep keys its chain cache on
the sequence of lock classes, so with a single class both orders hash
to the same chain and only whichever happens first is validated.

netdev_lock_cmp_fn() itself has another source of false-negatives.
lockdep calls it from within __lock_acquire(), with the recursion counter
already raised, so lock_is_held_type() always returns LOCK_STATE_UNKNOWN,
which means lockdep_rtnl_is_held() always returns true / held.
Use rtnl_is_locked(), which looks at the mutex directly and does
work from that context. We may still miss a bad case if some other
process is holding the lock, not us, but that's better than the
100% false negative rate of lockdep_rtnl_is_held().

One last thing, lockdep compares the address of the cmp function,
so it can't be a static inline - that would work similarly to having
separate classes, again. Move it to a source file.

Tested by nesting two instance locks from a module, one scenario per boot
since the first splat turns debug_locks off:

  first       second      rtnl      reported
  -----------------------------------------------------------
  virt->virt              no        recursive locking
  virt->virt              yes       -
  virt->phys              no        -
  virt->phys              yes       -
  phys->virt              no        - (records the edge)
  phys->virt              yes       - (records the edge)
  phys->phys              no        recursive locking
  phys->phys              yes       -
  virt->phys  phys->virt  no, no    circular dependency
  virt->phys  phys->virt  no, yes   circular dependency
  virt->phys  phys->virt  yes, no   circular dependency
  virt->phys  phys->virt  yes, yes  circular dependency
  phys->virt  virt->phys  no, no    circular dependency
  phys->virt  virt->phys  yes, yes  circular dependency
  virt->virt  virt->virt  yes, no   recursive locking
  phys->phys  phys->phys  yes, no   recursive locking

(netkit and dummy stood in for the virtual devices, virtio_net and
netdevsim for the physical ones.)

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20260926041832.1649675-1-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Jakub Kicinski 2026-09-25 21:18:31 -07:00 • committed by Paolo Abeni
commit b6f74dff6d
2 changed files with 40 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files