Re: [PATCH RFC 3/7] i3c: master: Add helper to query bus wakeup requirements

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `i3c_master_any_wakeup_enabled()` dereferences a dangling `desc->dev` pointer if a device previously failed to register.
- [High] Concurrent execution of `i3c_master_any_wakeup_enabled` and `i3c_master_reg_work_fn` creates a data race on `desc->dev` due to a shared read lock.
--

commit ed5405b56d89572a46136e7c93b53f6058fa4335
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Add helper to query bus wakeup requirements

Add i3c_master_any_wakeup_enabled(), which iterates over the devices on
an I3C bus and reports whether any of them are enabled for system wakeup.
Controller drivers can use this helper to determine whether wakeup support
must remain available while the system is suspended.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index a36f9b617cbf7..979457e635bd9 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1957,6 +1957,33 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
[ ... ]
> +bool i3c_master_any_wakeup_enabled(struct i3c_master_controller *master)
> +{
> +	struct i3c_dev_desc *desc;
> +	bool wakeup = false;
> +
> +	i3c_bus_normaluse_lock(&master->bus);

[Severity: High]
Can this race with i3c_master_reg_work_fn() and access uninitialized memory or
a freed pointer?

i3c_master_reg_work_fn() executes in a workqueue and calls
i3c_master_register_new_i3c_devs() while holding i3c_bus_normaluse_lock(),
which only acquires a read lock (down_read(&bus->lock)).

Inside i3c_master_register_new_i3c_devs(), desc->dev is allocated and mutated.

Since i3c_master_any_wakeup_enabled() also acquires the same read lock,
could it run concurrently (e.g. during runtime PM suspend) and read desc->dev
while it is being allocated or freed on the failure path?

> +	i3c_bus_for_each_i3cdev(&master->bus, desc) {
> +		if (desc->dev && device_may_wakeup(&desc->dev->dev)) {

[Severity: High]
Does this code risk a use-after-free if a device failed to register?

Looking at i3c_master_register_new_i3c_devs(), if device_register() fails,
the error path calls put_device(&desc->dev->dev) which frees the device
structure, but it leaves desc->dev as a dangling pointer.

If i3c_master_any_wakeup_enabled() is called later, could desc->dev evaluate
to true and cause device_may_wakeup() to dereference the freed memory?

> +			wakeup = true;
> +			break;
> +		}
> +	}
> +	i3c_bus_normaluse_unlock(&master->bus);
> +
> +	return wakeup;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_any_wakeup_enabled);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701200552.220042-1-adrian.hunter@intel.com?part=3

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c