* [PATCH 0/1] usb: ueagle-atm: wait for a firmware upload to complete
@ 2025-04-10 9:31 Andrey Tsygunka
2025-04-10 9:31 ` [PATCH 1/1] " Andrey Tsygunka
0 siblings, 1 reply; 3+ messages in thread
From: Andrey Tsygunka @ 2025-04-10 9:31 UTC (permalink / raw)
To: Matthieu CASTET, Stanislaw Gruszka
Cc: Andrey Tsygunka, Greg Kroah-Hartman, linux-usb, linux-kernel,
lvc-project
This suggested patch adds a fix to the ueagle-atm driver related to the
syzkaller report [1]. This report is currently relevant for older kernel
versions, WARN() is not shown in the upstream version due to its
replacement by pd_debug() in commit
d87c295f599cab2ab3b3df53a9098adba4a6002b in the sysfs subsystem, but it
reports a driver-side issue.
Hope this will be a useful fix.
[1] https://syzkaller.appspot.com/bug?extid=457452d30bcdda75ead2
Andrey Tsygunka (1):
usb: ueagle-atm: wait for a firmware upload to complete
drivers/usb/atm/ueagle-atm.c | 40 +++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] usb: ueagle-atm: wait for a firmware upload to complete
2025-04-10 9:31 [PATCH 0/1] usb: ueagle-atm: wait for a firmware upload to complete Andrey Tsygunka
@ 2025-04-10 9:31 ` Andrey Tsygunka
2025-04-12 18:53 ` Stanislaw Gruszka
0 siblings, 1 reply; 3+ messages in thread
From: Andrey Tsygunka @ 2025-04-10 9:31 UTC (permalink / raw)
To: Matthieu CASTET, Stanislaw Gruszka
Cc: Andrey Tsygunka, Greg Kroah-Hartman, linux-usb, linux-kernel,
lvc-project, stable
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>
---
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);
if (ret)
uea_err(usb, "firmware %s is not available\n", fw_name);
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] usb: ueagle-atm: wait for a firmware upload to complete
2025-04-10 9:31 ` [PATCH 1/1] " Andrey Tsygunka
@ 2025-04-12 18:53 ` Stanislaw Gruszka
0 siblings, 0 replies; 3+ messages in thread
From: Stanislaw Gruszka @ 2025-04-12 18:53 UTC (permalink / raw)
To: Andrey Tsygunka
Cc: Matthieu CASTET, Greg Kroah-Hartman, linux-usb, linux-kernel,
lvc-project, stable
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);
> if (ret)
> uea_err(usb, "firmware %s is not available\n", fw_name);
> @@ -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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-12 18:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10 9:31 [PATCH 0/1] usb: ueagle-atm: wait for a firmware upload to complete Andrey Tsygunka
2025-04-10 9:31 ` [PATCH 1/1] " Andrey Tsygunka
2025-04-12 18:53 ` Stanislaw Gruszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).