syzbot reported a circular locking dependency involving &net->xdp.lock,
&port->pnodes_lock, netdev_lock_ops(), and &xs->mutex:
-> #3 (&net->xdp.lock):
xsk_notifier+0x3d/0x2c0 net/xdp/xsk.c:2106
ipvlan_device_event+0x310/0x4e0 drivers/net/ipvlan/ipvlan_main.c:834
unregister_netdevice_many_notify+0x808/0x18b0 net/core/dev.c:12518
-> #2 (&port->pnodes_lock):
ipvlan_device_event+0x85/0x4e0 drivers/net/ipvlan/ipvlan_main.c:795
notifier_call_chain+0xb5/0x410 kernel/notifier.c:85
-> #1 (&dev_instance_lock_key / netdev_lock_ops):
netdev_lock_ops include/net/netdev_lock.h:42 [inline]
xsk_bind+0x331/0x11d0 net/xdp/xsk.c:1627
-> #0 (&xs->mutex):
xsk_diag_fill net/xdp/xsk_diag.c:113 [inline]
xsk_diag_dump+0x2e0/0x4e0 net/xdp/xsk_diag.c:166
The cycle exists through the following dependency chain:
1. xsk_bind() acquired netdev_lock_ops() while holding &xs->mutex (#1).
2. Device unregistration in ipvlan_device_event() acquired
&port->pnodes_lock (#2) and called xsk_notifier(), which acquired
&net->xdp.lock (#3).
3. Both xsk_diag_dump() and xsk_notifier() acquire &xs->mutex while
holding &net->xdp.lock (#0).
Break the circular dependency at its source in xsk_bind() by acquiring
netdev_lock_ops() before &xs->mutex. To preserve the exact errno precedence
(-EBUSY vs -ENODEV) without requiring the mutex to read sxdp_ifindex, we
look up the net_device first. If the device exists, we lock netdev_lock_ops()
and then &xs->mutex, evaluating the socket state safely. The cleanup path
is adjusted to unlock in reverse order, ensuring device references are
correctly maintained or dropped.
This single lock reordering fully eliminates the &xs->mutex ->
netdev_lock_ops() edge, resolving the 4-lock cycle without introducing
concurrency regressions in the diagnostic dump or notifier paths.
Fixes: 975b11ae9077 ("xsk: add socket allocate, create and bind")
Reported-by: syzbot+aa48b5fe7bfda62d1682@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=aa48b5fe7bfda62d1682
Signed-off-by: Khawar Ahemad <redacted>
---
v5 -> v6:
- Abandon the two-phase xsk_notifier() and xsk_diag_dump() decoupling from
v3-v5, which introduced a Use-After-Free during concurrent xsk_release(),
and an O(N^2) list traversal complexity in xsk_diag_dump().
- Break the cycle at its root in xsk_bind() by acquiring netdev_lock_ops()
before &xs->mutex, fully removing the &xs->mutex -> netdev_lock_ops edge.
- Look up the net_device before locking &xs->mutex to preserve the original
-EBUSY vs -ENODEV errno precedence without any behavioural change.
- Link to v5: https://lore.kernel.org/bpf/20260902041257.58374-1-ahemadkhawar123@gmail.com/
v4 -> v5:
- Rebase cleanly on latest bpf-next master.
- Link to v4: https://lore.kernel.org/bpf/20260826174744.3394-1-ahemadkhawar123@gmail.com/
v3 -> v4:
- Rebase cleanly on latest bpf-next master to resolve merge conflict.
- Update commit message to accurately describe the full 4-lock dependency
chain (&net->xdp.lock, &port->pnodes_lock, netdev_lock_ops, &xs->mutex).
- Link to v3: https://lore.kernel.org/bpf/20260826173019.2917-1-ahemadkhawar123@gmail.com/
v2 -> v3:
- Fix direct AB-BA lock inversion in xsk_notifier() by performing device
queue sweeps via xsk_get_pool_from_qid() outside &net->xdp.lock.
- Eliminate &net->xdp.lock -> &xs->mutex in xsk_diag_dump() by taking a
temporary socket reference and releasing the lock before xsk_diag_fill().
- Link to v2: https://lore.kernel.org/bpf/20260826162110.99879-1-ahemadkhawar123@gmail.com/
v1 -> v2:
- Avoid reordering locks in xsk_bind() to preserve errno precedence.
- Link to v1: https://lore.kernel.org/bpf/20260825152152.86092-1-ahemadkhawar123@gmail.com/
net/xdp/xsk.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
Does the commit message accurately describe the #2 lock acquisition?
The syzbot report shows the #2 acquisition of pnodes_lock comes from the
bonding/features path (NETDEV_FEAT_CHANGE event triggering
netdev_compute_master_upper_features), not from device unregistration.
The #3 stack shows unregister calling into ipvlan_device_event, but
that's a different event path.
The cycle exists through the following dependency chain:
1. xsk_bind() acquired netdev_lock_ops() while holding &xs->mutex (#1).
2. Device unregistration in ipvlan_device_event() acquired
&port->pnodes_lock (#2) and called xsk_notifier(), which acquired
&net->xdp.lock (#3).
3. Both xsk_diag_dump() and xsk_notifier() acquire &xs->mutex while
holding &net->xdp.lock (#0).
How does this enumeration form a cycle?
The enumerated items describe edges:
#0 -> #1: xs->mutex -> netdev_lock
#2 -> #3: pnodes_lock -> xdp.lock
#3 -> #0: xdp.lock -> xs->mutex
To close the cycle, we need the netdev_lock -> pnodes_lock edge (#1 -> #2),
but this edge is never mentioned in the enumeration. Without that fourth
edge, the description doesn't actually describe a complete cycle. The syzbot
report does contain this edge (via the bonding/features path in frame #2),
but the commit message omits it from the enumeration.
Could the commit message be clarified to explicitly state all four edges
that form the cycle?
[ ... ]
Is it appropriate to use the Closes tag here?
The syzbot dashboard shows this bug was already fixed by commit a8455260b2e9
("ipvlan: unregister upper devices outside pnodes_lock"), which removed the
pnodes_lock -> net->xdp.lock edge on the ipvlan side. That commit is present
in this tree at drivers/net/ipvlan/ipvlan_main.c:899-902.
Using the Closes tag on a redundant patch may cause syzbot and patchwork to
misattribute the fix to this commit instead of the actual fix.
Additionally, Maciej Fijalkowski informed you in v5 that they had already
fixed the issue on the ipvlan side:
https://lore.kernel.org/bpf/DM4PR11MB6117332DE5BD8408848E453D82B72@DM4PR11MB6117.namprd11.prod.outlook.com/
"We went with fixing ipvlan side instead:
https://lore.kernel.org/netdev/20260828164918.451364-1-maciej.fijalkowski@intel.com/
Forgot to CC you, sorry about that."
Does this XSK patch still serve a purpose given the ipvlan fix has already
resolved the reported issue?