Re: [PATCH v2] firmware: imx: scu-irq: drop wakeup_src sysfs file, log via dev_dbg()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The global device pointer `imx_sc_irq_dev` is cached without acquiring a reference, risking a Use-After-Free.
- [Medium] Race condition in `imx_sc_irq_dev` initialization exposes an uninitialized pointer to the work handler.
--

commit 3ac9bd2a1c44ca3562cddf96e23e9b5c60b0feea
Author: Stepan Ionichev <sozdayvek@gmail.com>

firmware: imx: scu-irq: drop wakeup_src sysfs file, log via dev_dbg()

This commit removes the undocumented sysfs attribute for waking up sources,
along with its supporting variables and kobject. It replaces the sysfs
interface with dynamic debug logging of the wakeup information via
dev_dbg() in the work handler.

Link: https://lore.kernel.org/all/2026051656-corral-edgy-290c@gregkh/

> diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c

[ ... ]

> @@ -233,29 +199,11 @@ int imx_scu_enable_general_irq_channel(struct device *dev)
>  	if (IS_ERR(ch)) {
>  		ret = PTR_ERR(ch);
>  		dev_err(dev, "failed to request mbox chan gip3, ret %d\n", ret);
> -		goto free_cl;
> -	}
> -
> -	/* Create directory under /sysfs/firmware */
> -	wakeup_obj = kobject_create_and_add("scu_wakeup_source", firmware_kobj);
> -	if (!wakeup_obj) {
> -		ret = -ENOMEM;
> -		goto free_ch;
> +		devm_kfree(dev, cl);
> +		return ret;
>  	}
>  
> -	ret = sysfs_create_file(wakeup_obj, &wakeup_source_attr.attr);
> -	if (ret) {
> -		dev_err(dev, "Cannot create wakeup source src file......\n");
> -		kobject_put(wakeup_obj);
> -		goto free_ch;
> -	}
> +	imx_sc_irq_dev = dev;

Can the work handler execute before imx_sc_irq_dev is initialized?

If mbox_request_channel_byname() successfully enables the channel right
before this block, an interrupt could fire immediately, invoke
imx_scu_irq_callback() and schedule the work.

If the work handler imx_scu_irq_work_handler() executes before the
assignment to imx_sc_irq_dev completes, dev_dbg() will receive a NULL
pointer. While __dev_printk() handles NULL gracefully by printing
"(NULL device *)", it would mask the intended debug output.

Should imx_sc_irq_dev be assigned before requesting the mailbox channel?

Does caching the global device pointer here also risk a use-after-free?

The dev pointer is stored in imx_sc_irq_dev without acquiring a reference.
Since the workqueue and mailbox channel aren't explicitly torn down on
failure or unbind, the pending asynchronous work might still run after the
device is removed.

If the device is freed, accessing it in imx_scu_irq_work_handler() via
dev_dbg() could lead to a use-after-free.

Would it be safer to take a reference with get_device() to guarantee the
device's lifetime during asynchronous execution?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518093934.1910-1-sozdayvek@gmail.com?part=1