[PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[syzbot]

From: Slawomir Stepien <sst@poczta.fm>

There is an ABBA deadlock between the devlink instance lock and the debugfs
active file reference mechanism.

When a device is being removed (e.g., via nsim_drv_remove()), the driver
acquires the devlink instance lock and then calls
debugfs_remove_recursive(). This function blocks waiting for all active
file operations on the debugfs files to complete.

Concurrently, if a user writes to the "max_vfs" debugfs file, the VFS layer
acquires an active reference to the file. The write handler,
nsim_bus_dev_max_vfs_write(), then attempts to acquire the devlink instance
lock, which is already held by the removal task.

This creates a circular dependency resulting in a deadlock:

INFO: task blocked for more than 143 seconds.
Call Trace:
 wait_for_completion+0x2ca/0x5e0 kernel/sched/completion.c:153
 __debugfs_file_removed fs/debugfs/inode.c:751 [inline]
 remove_one+0x2df/0x3b0 fs/debugfs/inode.c:758
 __simple_recursive_removal+0x215/0x520 fs/libfs.c:623
 debugfs_remove+0x5b/0x70 fs/debugfs/inode.c:781
 nsim_dev_debugfs_exit drivers/net/netdevsim/dev.c:372 [inline]
 nsim_drv_remove+0xc0/0x170 drivers/net/netdevsim/dev.c:1803

INFO: task blocked for more than 143 seconds.
Call Trace:
 __mutex_lock+0x7bf/0x1550 kernel/locking/mutex.c:821
 nsim_bus_dev_max_vfs_write+0x229/0x3d0 drivers/net/netdevsim/dev.c:276
 full_proxy_write+0x127/0x1f0 fs/debugfs/file.c:388
 vfs_write+0x296/0xba0 fs/read_write.c:685

To fix this, use devl_trylock() in nsim_bus_dev_max_vfs_write() instead of
devl_lock(). If the lock cannot be acquired, return -EBUSY. This aborts the
write operation, releases the debugfs active file reference, and allows the
pending debugfs_remove_recursive() to proceed.

Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+3147c5de186107ffc7a1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3147c5de186107ffc7a1
Link: https://syzkaller.appspot.com/ai_job?id=fcdda182-bebf-49ab-ada0-d4d2e814ebf7
Signed-off-by: Slawomir Stepien <sst@poczta.fm>

---
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index aed9ad5f1..421cd7327 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -273,7 +273,11 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
 		return -ENOMEM;
 
 	nsim_dev = file->private_data;
-	devl_lock(priv_to_devlink(nsim_dev));
+	if (!devl_trylock(priv_to_devlink(nsim_dev))) {
+		ret = -EBUSY;
+		goto out;
+	}
+
 	/* Reject if VFs are configured */
 	if (nsim_dev_get_vfs(nsim_dev)) {
 		ret = -EBUSY;
@@ -285,6 +289,7 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
 	}
 	devl_unlock(priv_to_devlink(nsim_dev));
 
+out:
 	kfree(vfconfigs);
 	return ret;
 }


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
You can comment on the patch as usual, syzbot will try to address
the comments and send a new version of the patch if necessary.
syzbot engineers can be reached at syzkaller@googlegroups.com.

Re: [PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[Paolo Abeni]

On 7/31/26 11:55 AM, syzbot wrote:
> From: Slawomir Stepien <sst@poczta.fm>
> 
> There is an ABBA deadlock between the devlink instance lock and the debugfs
> active file reference mechanism.
> 
> When a device is being removed (e.g., via nsim_drv_remove()), the driver
> acquires the devlink instance lock and then calls
> debugfs_remove_recursive(). This function blocks waiting for all active
> file operations on the debugfs files to complete.
> 
> Concurrently, if a user writes to the "max_vfs" debugfs file, the VFS layer
> acquires an active reference to the file. The write handler,
> nsim_bus_dev_max_vfs_write(), then attempts to acquire the devlink instance
> lock, which is already held by the removal task.
> 
> This creates a circular dependency resulting in a deadlock:
> 
> INFO: task blocked for more than 143 seconds.
> Call Trace:
>  wait_for_completion+0x2ca/0x5e0 kernel/sched/completion.c:153
>  __debugfs_file_removed fs/debugfs/inode.c:751 [inline]
>  remove_one+0x2df/0x3b0 fs/debugfs/inode.c:758
>  __simple_recursive_removal+0x215/0x520 fs/libfs.c:623
>  debugfs_remove+0x5b/0x70 fs/debugfs/inode.c:781
>  nsim_dev_debugfs_exit drivers/net/netdevsim/dev.c:372 [inline]
>  nsim_drv_remove+0xc0/0x170 drivers/net/netdevsim/dev.c:1803
> 
> INFO: task blocked for more than 143 seconds.
> Call Trace:
>  __mutex_lock+0x7bf/0x1550 kernel/locking/mutex.c:821
>  nsim_bus_dev_max_vfs_write+0x229/0x3d0 drivers/net/netdevsim/dev.c:276
>  full_proxy_write+0x127/0x1f0 fs/debugfs/file.c:388
>  vfs_write+0x296/0xba0 fs/read_write.c:685
> 
> To fix this, use devl_trylock() in nsim_bus_dev_max_vfs_write() instead of
> devl_lock(). If the lock cannot be acquired, return -EBUSY. This aborts the
> write operation, releases the debugfs active file reference, and allows the
> pending debugfs_remove_recursive() to proceed.
> 
> Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
> Reported-by: syzbot+3147c5de186107ffc7a1@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3147c5de186107ffc7a1
> Link: https://syzkaller.appspot.com/ai_job?id=fcdda182-bebf-49ab-ada0-d4d2e814ebf7
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> 
> ---
> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index aed9ad5f1..421cd7327 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -273,7 +273,11 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
>  		return -ENOMEM;
>  
>  	nsim_dev = file->private_data;
> -	devl_lock(priv_to_devlink(nsim_dev));
> +	if (!devl_trylock(priv_to_devlink(nsim_dev))) {
> +		ret = -EBUSY;

I think this should return restart_syscall(). This kind of schema is
calling for trouble, but for netdevsim should be okish.

/P

Re: [PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[Slawomir Stepien]

On sie 04, 2026 16:33, Paolo Abeni wrote:
> On 7/31/26 11:55 AM, syzbot wrote:
> > From: Slawomir Stepien <sst@poczta.fm>
> > 
> > There is an ABBA deadlock between the devlink instance lock and the debugfs
> > active file reference mechanism.
> > 
> > When a device is being removed (e.g., via nsim_drv_remove()), the driver
> > acquires the devlink instance lock and then calls
> > debugfs_remove_recursive(). This function blocks waiting for all active
> > file operations on the debugfs files to complete.
> > 
> > Concurrently, if a user writes to the "max_vfs" debugfs file, the VFS layer
> > acquires an active reference to the file. The write handler,
> > nsim_bus_dev_max_vfs_write(), then attempts to acquire the devlink instance
> > lock, which is already held by the removal task.
> > 
> > This creates a circular dependency resulting in a deadlock:
> > 
> > INFO: task blocked for more than 143 seconds.
> > Call Trace:
> >  wait_for_completion+0x2ca/0x5e0 kernel/sched/completion.c:153
> >  __debugfs_file_removed fs/debugfs/inode.c:751 [inline]
> >  remove_one+0x2df/0x3b0 fs/debugfs/inode.c:758
> >  __simple_recursive_removal+0x215/0x520 fs/libfs.c:623
> >  debugfs_remove+0x5b/0x70 fs/debugfs/inode.c:781
> >  nsim_dev_debugfs_exit drivers/net/netdevsim/dev.c:372 [inline]
> >  nsim_drv_remove+0xc0/0x170 drivers/net/netdevsim/dev.c:1803
> > 
> > INFO: task blocked for more than 143 seconds.
> > Call Trace:
> >  __mutex_lock+0x7bf/0x1550 kernel/locking/mutex.c:821
> >  nsim_bus_dev_max_vfs_write+0x229/0x3d0 drivers/net/netdevsim/dev.c:276
> >  full_proxy_write+0x127/0x1f0 fs/debugfs/file.c:388
> >  vfs_write+0x296/0xba0 fs/read_write.c:685
> > 
> > To fix this, use devl_trylock() in nsim_bus_dev_max_vfs_write() instead of
> > devl_lock(). If the lock cannot be acquired, return -EBUSY. This aborts the
> > write operation, releases the debugfs active file reference, and allows the
> > pending debugfs_remove_recursive() to proceed.
> > 
> > Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
> > Reported-by: syzbot+3147c5de186107ffc7a1@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3147c5de186107ffc7a1
> > Link: https://syzkaller.appspot.com/ai_job?id=fcdda182-bebf-49ab-ada0-d4d2e814ebf7
> > Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> > 
> > ---
> > diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> > index aed9ad5f1..421cd7327 100644
> > --- a/drivers/net/netdevsim/dev.c
> > +++ b/drivers/net/netdevsim/dev.c
> > @@ -273,7 +273,11 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
> >  		return -ENOMEM;
> >  
> >  	nsim_dev = file->private_data;
> > -	devl_lock(priv_to_devlink(nsim_dev));
> > +	if (!devl_trylock(priv_to_devlink(nsim_dev))) {
> > +		ret = -EBUSY;
> 
> I think this should return restart_syscall(). This kind of schema is
> calling for trouble, but for netdevsim should be okish.

Thanks Paolo for your comment!

Can you explain a bit more? What is your reasoning here? Is it, that we expect that we could have
the lock in e.g. 1ns, so it is worth restarting without bothering userspace[1]?

I see a lot of:

if (!rtnl_trylock())
	return restart_syscall();

Why this pattern is so popular? Is it for the same reason as above?

[1] https://kernel-internals.org/syscalls/restart-block/

-- 
Slawomir Stepien

Re: [PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[Paolo Abeni]

On 8/5/26 10:55 AM, Slawomir Stepien wrote:
> On sie 04, 2026 16:33, Paolo Abeni wrote:
>> On 7/31/26 11:55 AM, syzbot wrote:
>>> From: Slawomir Stepien <sst@poczta.fm>
>>>
>>> There is an ABBA deadlock between the devlink instance lock and the debugfs
>>> active file reference mechanism.
>>>
>>> When a device is being removed (e.g., via nsim_drv_remove()), the driver
>>> acquires the devlink instance lock and then calls
>>> debugfs_remove_recursive(). This function blocks waiting for all active
>>> file operations on the debugfs files to complete.
>>>
>>> Concurrently, if a user writes to the "max_vfs" debugfs file, the VFS layer
>>> acquires an active reference to the file. The write handler,
>>> nsim_bus_dev_max_vfs_write(), then attempts to acquire the devlink instance
>>> lock, which is already held by the removal task.
>>>
>>> This creates a circular dependency resulting in a deadlock:
>>>
>>> INFO: task blocked for more than 143 seconds.
>>> Call Trace:
>>>  wait_for_completion+0x2ca/0x5e0 kernel/sched/completion.c:153
>>>  __debugfs_file_removed fs/debugfs/inode.c:751 [inline]
>>>  remove_one+0x2df/0x3b0 fs/debugfs/inode.c:758
>>>  __simple_recursive_removal+0x215/0x520 fs/libfs.c:623
>>>  debugfs_remove+0x5b/0x70 fs/debugfs/inode.c:781
>>>  nsim_dev_debugfs_exit drivers/net/netdevsim/dev.c:372 [inline]
>>>  nsim_drv_remove+0xc0/0x170 drivers/net/netdevsim/dev.c:1803
>>>
>>> INFO: task blocked for more than 143 seconds.
>>> Call Trace:
>>>  __mutex_lock+0x7bf/0x1550 kernel/locking/mutex.c:821
>>>  nsim_bus_dev_max_vfs_write+0x229/0x3d0 drivers/net/netdevsim/dev.c:276
>>>  full_proxy_write+0x127/0x1f0 fs/debugfs/file.c:388
>>>  vfs_write+0x296/0xba0 fs/read_write.c:685
>>>
>>> To fix this, use devl_trylock() in nsim_bus_dev_max_vfs_write() instead of
>>> devl_lock(). If the lock cannot be acquired, return -EBUSY. This aborts the
>>> write operation, releases the debugfs active file reference, and allows the
>>> pending debugfs_remove_recursive() to proceed.
>>>
>>> Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
>>> Reported-by: syzbot+3147c5de186107ffc7a1@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=3147c5de186107ffc7a1
>>> Link: https://syzkaller.appspot.com/ai_job?id=fcdda182-bebf-49ab-ada0-d4d2e814ebf7
>>> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
>>>
>>> ---
>>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>>> index aed9ad5f1..421cd7327 100644
>>> --- a/drivers/net/netdevsim/dev.c
>>> +++ b/drivers/net/netdevsim/dev.c
>>> @@ -273,7 +273,11 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
>>>  		return -ENOMEM;
>>>  
>>>  	nsim_dev = file->private_data;
>>> -	devl_lock(priv_to_devlink(nsim_dev));
>>> +	if (!devl_trylock(priv_to_devlink(nsim_dev))) {
>>> +		ret = -EBUSY;
>>
>> I think this should return restart_syscall(). This kind of schema is
>> calling for trouble, but for netdevsim should be okish.
> 
> Thanks Paolo for your comment!
> 
> Can you explain a bit more? What is your reasoning here? Is it, that we expect that we could have
> the lock in e.g. 1ns, so it is worth restarting without bothering userspace[1]?
> 
> I see a lot of:
> 
> if (!rtnl_trylock())
> 	return restart_syscall();
> 
> Why this pattern is so popular? Is it for the same reason as above?
> 
> [1] https://kernel-internals.org/syscalls/restart-block/
If you don't restart, the write will randomly fail when the lock is
contended, as reported by sashiko gemini:

https://sashiko.dev/#/patchset/b7bf56ea-7522-4163-acd5-aaa69ad03b3a%40mail.kernel.org

the user-experience will be terrible at best.

Also, I missed this other report initially:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/b7bf56ea-7522-4163-acd5-aaa69ad03b3a%40mail.kernel.org

it looks like the above is not a complete fix.

/P

Re: [PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[Jakub Kicinski]

On Fri, 31 Jul 2026 09:55:04 +0000 (UTC) syzbot wrote:
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -273,7 +273,11 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,

Sorry to chime in poniewczasie.
Please delete this file. netdevsim is a harness for in-tree selftests,
none seems to be reading or writing the debugfs num_vfs AFAICT
so let's just drop it?

Re: [PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[Slawomir Stepien]

On sie 05, 2026 16:08, Jakub Kicinski wrote:
> On Fri, 31 Jul 2026 09:55:04 +0000 (UTC) syzbot wrote:
> > --- a/drivers/net/netdevsim/dev.c
> > +++ b/drivers/net/netdevsim/dev.c
> > @@ -273,7 +273,11 @@ static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
> 
> Sorry to chime in poniewczasie.
> Please delete this file. netdevsim is a harness for in-tree selftests,
> none seems to be reading or writing the debugfs num_vfs AFAICT
> so let's just drop it?

That's true, but is netdevsim used *just* by the selftests (or was designed with only selftests in
mind)? What if someone is using it without selftests?

On the other hand: what sashiko found:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/b7bf56ea-7522-4163-acd5-aaa69ad03b3a%40mail.kernel.org
is true: the same issue will be with e.g. break_health. So it seems to me that a better approach
would be to change when the debugfs files are removed.

It seems to me that change in nsim_drv_remove() might be easy, but what about
nsim_dev_reload_down()...it seems it will have the same deadlock. Or am I missing something for this
case?

-- 
Slawomir Stepien

Re: [PATCH] netdevsim: fix deadlock in nsim_bus_dev_max_vfs_write()

[Jakub Kicinski]

On Fri, 7 Aug 2026 09:31:20 +0200 Slawomir Stepien wrote:
> That's true, but is netdevsim used *just* by the selftests (or was
> designed with only selftests in mind)? What if someone is using it
> without selftests?

Quoting documentation:

  netdevsim
  ~~~~~~~~~
  
  ``netdevsim`` is a test driver which can be used to exercise driver
  configuration APIs without requiring capable hardware.
  Mock-ups and tests based on ``netdevsim`` are encouraged when
  adding new APIs with complex logic in the stack. The tests should
  be written so that they can run both against ``netdevsim`` and a real
  device (see ``tools/testing/selftests/drivers/net/README.rst``).
  ``netdevsim``-only tests should focus on testing corner cases
  and failure paths in the core which are hard to exercise with a real driver.
  
  ``netdevsim`` in itself is **not** considered
  a use case/user. You must also implement the new APIs in a real driver.
  
  We give no guarantees that ``netdevsim`` won't change in the future
  in a way which would break what would normally be considered uAPI.
  
  ``netdevsim`` is reserved for use by upstream tests only, so any
  new ``netdevsim`` features must be accompanied by selftests under
  ``tools/testing/selftests/``.
  
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#netdevsim

> On the other hand: what sashiko found:
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/b7bf56ea-7522-4163-acd5-aaa69ad03b3a%40mail.kernel.org
> is true: the same issue will be with e.g. break_health. So it seems
> to me that a better approach would be to change when the debugfs
> files are removed.

I seem to recall being annoyed at the fact that the health API takes
devlink lock. It should be callable from IRQ even. Forcing drivers
to worry about calling context is annoying for real drivers too.

> It seems to me that change in nsim_drv_remove() might be easy, but
> what about nsim_dev_reload_down()...it seems it will have the same
> deadlock. Or am I missing something for this case?

netdevsim is just a test mock. Fixing it for the sake of fixing
netdevsim is a waste of everyone's time. The first question you should
be asking yourself is "do I understand what this code was *designed
for*"..