On Thu, Apr 10, 2025 at 12:31:46PM +0300, Andrey Tsygunka wrote:
Syzkaller reported:
sysfs group 'power' not found for kobject 'ueagle-atm!adi930.fw' WARNING: CPU: 1 PID: 6804 at fs/sysfs/group.c:278 sysfs_remove_group+0x120/0x170 fs/sysfs/group.c:278 Modules linked in: CPU: 1 PID: 6804 Comm: kworker/1:5 Not tainted 6.1.128 #1 Hardware name: linux,dummy-virt (DT) Workqueue: events request_firmware_work_func Call trace: sysfs_remove_group+0x120/0x170 fs/sysfs/group.c:278 dpm_sysfs_remove+0x9c/0xc0 drivers/base/power/sysfs.c:837 device_del+0x1e0/0xb30 drivers/base/core.c:3861 fw_load_sysfs_fallback drivers/base/firmware_loader/fallback.c:120 [inline] fw_load_from_user_helper drivers/base/firmware_loader/fallback.c:158 [inline] firmware_fallback_sysfs+0x880/0xa30 drivers/base/firmware_loader/fallback.c:234 _request_firmware+0xcc0/0x1030 drivers/base/firmware_loader/main.c:884 request_firmware_work_func+0xf0/0x240 drivers/base/firmware_loader/main.c:1135 process_one_work+0x878/0x1770 kernel/workqueue.c:2292 worker_thread+0x48c/0xe40 kernel/workqueue.c:2439 kthread+0x274/0x2e0 kernel/kthread.c:376 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:864
When calling the usb-device probe() method, request_firmware_nowait() is called, an async task is created that creates a child device to load the firmware and waits (fw_sysfs_wait_timeout()) for the firmware to be ready. If an async disconnect event occurs for usb-device while waiting, we may get a WARN() when calling firmware_fallback_sysfs() about "no sysfs group 'power' found for kobject" because it was removed by usb_disconnect().
To avoid this, add a routine to wait for the firmware loading process to complete to prevent premature device disconnection.
Fixes: b72458a80c75 ("[PATCH] USB: Eagle and ADI 930 usb adsl modem driver") Cc: stable@vger.kernel.org Signed-off-by: Andrey Tsygunka aitsygunka@yandex.ru
Hi, thanks for the patch, but I do not see benefit of fix ex-WARN and now pr_debug(). Only downside of adding extra 40 lines of code.
Nacked-by: Stanislaw Gruszka stf_xl@wp.pl
drivers/usb/atm/ueagle-atm.c | 40 +++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c index cd0f7b4bd82a..eaa5ad316d89 100644 --- a/drivers/usb/atm/ueagle-atm.c +++ b/drivers/usb/atm/ueagle-atm.c @@ -570,6 +570,12 @@ MODULE_PARM_DESC(annex, #define LOAD_INTERNAL 0xA0 #define F8051_USBCS 0x7f92 +struct uea_interface_data {
- struct completion fw_upload_complete;
- struct usb_device *usb;
- struct usb_interface *intf;
+};
/*
- uea_send_modem_cmd - Send a command for pre-firmware devices.
*/ @@ -599,7 +605,8 @@ static int uea_send_modem_cmd(struct usb_device *usb, static void uea_upload_pre_firmware(const struct firmware *fw_entry, void *context) {
- struct usb_device *usb = context;
- struct uea_interface_data *uea_intf_data = context;
- struct usb_device *usb = uea_intf_data->usb; const u8 *pfw; u8 value; u32 crc = 0;
@@ -669,15 +676,17 @@ static void uea_upload_pre_firmware(const struct firmware *fw_entry, uea_err(usb, "firmware is corrupted\n"); err: release_firmware(fw_entry);
- complete(&uea_intf_data->fw_upload_complete); uea_leaves(usb);
} /*
- uea_load_firmware - Load usb firmware for pre-firmware devices.
*/ -static int uea_load_firmware(struct usb_device *usb, unsigned int ver) +static int uea_load_firmware(struct uea_interface_data *uea_intf_data, unsigned int ver) { int ret;
- struct usb_device *usb = uea_intf_data->usb; char *fw_name = EAGLE_FIRMWARE;
uea_enters(usb); @@ -702,7 +711,7 @@ static int uea_load_firmware(struct usb_device *usb, unsigned int ver) } ret = request_firmware_nowait(THIS_MODULE, 1, fw_name, &usb->dev,
GFP_KERNEL, usb,
GFP_KERNEL, uea_intf_data, uea_upload_pre_firmware);@@ -2586,6 +2595,7 @@ static struct usbatm_driver uea_usbatm_driver = { static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_device *usb = interface_to_usbdev(intf);
- struct uea_interface_data *uea_intf_data; int ret;
uea_enters(usb); @@ -2597,8 +2607,23 @@ static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id) usb_reset_device(usb);
- if (UEA_IS_PREFIRM(id))
return uea_load_firmware(usb, UEA_CHIP_VERSION(id));
- if (UEA_IS_PREFIRM(id)) {
uea_intf_data = devm_kzalloc(&usb->dev, sizeof(*uea_intf_data), GFP_KERNEL);if (!uea_intf_data)return -ENOMEM;init_completion(&uea_intf_data->fw_upload_complete);uea_intf_data->usb = usb;uea_intf_data->intf = intf;usb_set_intfdata(intf, uea_intf_data);ret = uea_load_firmware(uea_intf_data, UEA_CHIP_VERSION(id));if (ret)complete(&uea_intf_data->fw_upload_complete);return ret;- }
ret = usbatm_usb_probe(intf, id, &uea_usbatm_driver); if (ret == 0) { @@ -2618,6 +2643,7 @@ static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id) static void uea_disconnect(struct usb_interface *intf) { struct usb_device *usb = interface_to_usbdev(intf);
- struct uea_interface_data *uea_intf_data; int ifnum = intf->altsetting->desc.bInterfaceNumber; uea_enters(usb);
@@ -2629,6 +2655,10 @@ static void uea_disconnect(struct usb_interface *intf) usbatm_usb_disconnect(intf); mutex_unlock(&uea_mutex); uea_info(usb, "ADSL device removed\n");
- } else {
uea_intf_data = usb_get_intfdata(intf);uea_info(usb, "wait for completion uploading firmware\n");wait_for_completion(&uea_intf_data->fw_upload_complete);uea_leaves(usb); -- 2.25.1