[PATCH 1/7] firmware_loader: Stop pinning modules on registration
From: Pratik R. Sampat <hidden>
Date: 2026-09-10 17:03:34
Also in:
lkml
Subsystem:
driver core, kobjects, debugfs and sysfs, firmware loader (request_firmware), the rest · Maintainers:
Greg Kroah-Hartman, "Rafael J. Wysocki", Danilo Krummrich, Luis Chamberlain, Linus Torvalds
From: Dan Williams <djbw@kernel.org> The module reference counting can result in callers pinning themselves in a circular loop. The module reference counting is unnecessary. firmware_upload_unregister() must be able to guarantee that all ops are idle at return. All ops are either called from sysfs or the workqueue, so unregister sysfs to stop submissions, cancel any started transfers, flush cancelled transfers, and then release the device. This also solves a theoretical race of new submissions starting between flush_work() and device_unregister(). The module reference was not protecting against that race. Cc: Luis Chamberlain <mcgrof@kernel.org> Cc: Russ Weight <redacted> Cc: Danilo Krummrich <dakr@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Reported-by: Chao Gao <redacted> Tested-by: Chao Gao <redacted> Closes: https://sashiko.dev/#/patchset/20260326084448.29947-1-chao.gao%40intel.com?patch=10705 Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support") Signed-off-by: Dan Williams <djbw@kernel.org> Signed-off-by: Pratik R. Sampat <redacted> --- drivers/base/firmware_loader/sysfs_upload.c | 31 ++++++++++----------- 1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index efc33294212f..6f2123538682 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c@@ -312,14 +312,9 @@ firmware_upload_register(struct module *module, struct device *parent, return ERR_PTR(-EINVAL); } - if (!try_module_get(module)) - return ERR_PTR(-EFAULT); - fw_upload = kzalloc_obj(*fw_upload); - if (!fw_upload) { - ret = -ENOMEM; - goto exit_module_put; - } + if (!fw_upload) + return ERR_PTR(-ENOMEM); fw_upload_priv = kzalloc_obj(*fw_upload_priv); if (!fw_upload_priv) {
@@ -361,7 +356,7 @@ firmware_upload_register(struct module *module, struct device *parent, if (ret) { dev_err(fw_dev, "%s: device_register failed\n", __func__); put_device(fw_dev); - goto exit_module_put; + return ERR_PTR(ret); } return fw_upload;
@@ -372,9 +367,6 @@ firmware_upload_register(struct module *module, struct device *parent, free_fw_upload: kfree(fw_upload); -exit_module_put: - module_put(module); - return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(firmware_upload_register);
@@ -386,23 +378,28 @@ EXPORT_SYMBOL_GPL(firmware_upload_register); void firmware_upload_unregister(struct fw_upload *fw_upload) { struct fw_sysfs *fw_sysfs = fw_upload->priv; + struct device *parent = fw_sysfs->dev.parent; struct fw_upload_priv *fw_upload_priv = fw_sysfs->fw_upload_priv; - struct module *module = fw_upload_priv->module; + + /* hold a parent reference while child is unregistered */ + get_device(parent); + + /* shutdown the sysfs interface to block new requests */ + device_del(&fw_sysfs->dev); mutex_lock(&fw_upload_priv->lock); if (fw_upload_priv->progress == FW_UPLOAD_PROG_IDLE) { mutex_unlock(&fw_upload_priv->lock); - goto unregister; + goto release; } fw_upload_priv->ops->cancel(fw_upload); mutex_unlock(&fw_upload_priv->lock); +release: /* Ensure lower-level device-driver is finished */ flush_work(&fw_upload_priv->work); - -unregister: - device_unregister(&fw_sysfs->dev); - module_put(module); + put_device(&fw_sysfs->dev); + put_device(parent); } EXPORT_SYMBOL_GPL(firmware_upload_unregister);
--
2.43.0