[PATCH net v2] devlink: request flash firmware without instance lock

[PATCH net v2] devlink: request flash firmware without instance lock

[Miguel Garcia]

request_firmware() may enter the userspace fallback and call
try_to_freeze(). Holding the devlink instance lock across that call
triggers a lockdep warning and can block unregister for the duration of
the firmware fallback.

Drop the instance lock around firmware loading in both flash update paths.
The callers hold a devlink reference, which also pins the parent device.
Recheck registration after reacquiring the lock before calling into the
driver.

Fixes: b44cfd4f5b91 ("devlink: move request_firmware out of driver")
Reported-by: syzbot+372a7d84708b07f64d9b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=372a7d84708b07f64d9b
Signed-off-by: Miguel Garcia <miguelgarciaroman8@gmail.com>
---
Changes in v2:
- Cc Jacob Keller, author of b44cfd4f5b91, as requested by Andrew Lunn.
- Keep only b44cfd4f5b91 as the Fixes tag; ed539ba614a0 provides the
  registration check but did not introduce the locking bug.

v1: https://lore.kernel.org/r/20260829142525.3900565-1-miguelgarciaroman8@gmail.com

 net/devlink/dev.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 55959b0ff..cdbf39f19 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -1169,17 +1169,25 @@ int devlink_nl_flash_update_doit(struct sk_buff *skb, struct genl_info *info)
 
 	nla_file_name = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME];
 	file_name = nla_data(nla_file_name);
+	devl_unlock(devlink);
 	ret = request_firmware(&params.fw, file_name, devlink->dev);
+	devl_lock(devlink);
 	if (ret) {
 		NL_SET_ERR_MSG_ATTR(info->extack, nla_file_name,
 				    "failed to locate the requested firmware file");
 		return ret;
 	}
+	/* The device may have been unregistered while the lock was dropped. */
+	if (!devl_is_registered(devlink)) {
+		ret = -ENODEV;
+		goto out_release;
+	}
 
 	devlink_flash_update_begin_notify(devlink);
 	ret = devlink->ops->flash_update(devlink, &params, info->extack);
 	devlink_flash_update_end_notify(devlink);
 
+out_release:
 	release_firmware(params.fw);
 
 	return ret;
@@ -1244,15 +1252,24 @@ int devlink_compat_flash_update(struct devlink *devlink, const char *file_name)
 		ret = -EOPNOTSUPP;
 		goto out_unlock;
 	}
+	devl_unlock(devlink);
 
 	ret = request_firmware(&params.fw, file_name, devlink->dev);
+	devl_lock(devlink);
 	if (ret)
 		goto out_unlock;
 
+	/* The device may have been unregistered while the lock was dropped. */
+	if (!devl_is_registered(devlink)) {
+		ret = -ENODEV;
+		goto out_release;
+	}
+
 	devlink_flash_update_begin_notify(devlink);
 	ret = devlink->ops->flash_update(devlink, &params, NULL);
 	devlink_flash_update_end_notify(devlink);
 
+out_release:
 	release_firmware(params.fw);
 out_unlock:
 	devl_unlock(devlink);
-- 
2.43.0

Re: [PATCH net v2] devlink: request flash firmware without instance lock

[Jakub Kicinski]

On Sun, 30 Aug 2026 13:17:00 +0200 Miguel Garcia wrote:
> request_firmware() may enter the userspace fallback and call
> try_to_freeze(). Holding the devlink instance lock across that call
> triggers a lockdep warning and can block unregister for the duration of
> the firmware fallback.
> 
> Drop the instance lock around firmware loading in both flash update paths.
> The callers hold a devlink reference, which also pins the parent device.
> Recheck registration after reacquiring the lock before calling into the
> driver.

How did you find this issue? Nobody actually uses the user space
fallback, AFAIK? I'd just switch this to request_firmware_direct().
-- 
pw-bot: cr

Re: [PATCH net v2] devlink: request flash firmware without instance lock

[Jakub Kicinski]

On Sun, 30 Aug 2026 14:01:57 -0700 Jakub Kicinski wrote:
> On Sun, 30 Aug 2026 13:17:00 +0200 Miguel Garcia wrote:
> > request_firmware() may enter the userspace fallback and call
> > try_to_freeze(). Holding the devlink instance lock across that call
> > triggers a lockdep warning and can block unregister for the duration of
> > the firmware fallback.
> > 
> > Drop the instance lock around firmware loading in both flash update paths.
> > The callers hold a devlink reference, which also pins the parent device.
> > Recheck registration after reacquiring the lock before calling into the
> > driver.  
> 
> How did you find this issue?

Ah, I missed the syzbot link. Sigh.

[PATCH net v3] devlink: use direct firmware requests for flash updates

[Miguel Garcia]

request_firmware() may enter the sysfs fallback and call
try_to_freeze(). Devlink invokes it while holding the instance lock,
causing syzbot to report:

  WARNING: syz-executor/... still has locks held!

Firmware flash requests already name a file provided by userspace.
Use request_firmware_direct() in both flash update paths so a missing
file fails immediately instead of entering the sysfs fallback. This
keeps the normal devlink locking intact.

On systems with CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y, devlink flash
can no longer obtain a missing image through that fallback. Callers still
receive the existing error result, and netlink users retain the extack
message.

Fixes: b44cfd4f5b91 ("devlink: move request_firmware out of driver")
Reported-by: syzbot+372a7d84708b07f64d9b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=372a7d84708b07f64d9b
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Miguel Garcia <miguelgarciaroman8@gmail.com>
---
Changes in v3:
- Use request_firmware_direct() instead of dropping the devlink instance
  lock, as suggested by Jakub Kicinski.
- Document the resulting sysfs fallback behavior.
- Rebase onto net/main at bc93419130bb.

Changes in v2:
- Cc Jacob Keller, author of b44cfd4f5b91, as requested by Andrew Lunn.
- Keep only b44cfd4f5b91 as the Fixes tag; ed539ba614a0 provides the
  registration check but did not introduce the locking bug.

v2: https://lore.kernel.org/r/20260830111700.1799255-1-miguelgarciaroman8@gmail.com
v1: https://lore.kernel.org/r/20260829142525.3900565-1-miguelgarciaroman8@gmail.com

 net/devlink/dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 55959b0ff..b85bb02a9 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -1169,7 +1169,7 @@ int devlink_nl_flash_update_doit(struct sk_buff *skb, struct genl_info *info)
 
 	nla_file_name = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME];
 	file_name = nla_data(nla_file_name);
-	ret = request_firmware(&params.fw, file_name, devlink->dev);
+	ret = request_firmware_direct(&params.fw, file_name, devlink->dev);
 	if (ret) {
 		NL_SET_ERR_MSG_ATTR(info->extack, nla_file_name,
 				    "failed to locate the requested firmware file");
@@ -1245,7 +1245,7 @@ int devlink_compat_flash_update(struct devlink *devlink, const char *file_name)
 		goto out_unlock;
 	}
 
-	ret = request_firmware(&params.fw, file_name, devlink->dev);
+	ret = request_firmware_direct(&params.fw, file_name, devlink->dev);
 	if (ret)
 		goto out_unlock;
 
-- 
2.43.0

Re: [PATCH net v3] devlink: use direct firmware requests for flash updates

[Andrew Lunn]

On Tue, Sep 01, 2026 at 02:38:19PM +0200, Miguel Garcia wrote:
> request_firmware() may enter the sysfs fallback and call
> try_to_freeze(). Devlink invokes it while holding the instance lock,
> causing syzbot to report:

Please always start a new thread for a new patch. The CI system does
not always pick up the new version and test it otherwise.


    Andrew

---
pw-bot: cr

Re: [PATCH net v3] devlink: use direct firmware requests for flash updates

[Miguel Garcia]

Thanks, I've resent v4 as a new thread.