Re: [PATCH V2 3/8] i3c: master: Report wakeup events for IBIs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Concurrent execution of `i3c_master_queue_ibi()` and device unregistration leads to a NULL pointer dereference or Use-After-Free on `dev->dev`.

Pre-existing issues:
- [High] Concurrently accessing `dev->ibi` and its workqueue without synchronization leads to Use-After-Free.
--

--- Patch [3]: [PATCH V2 3/8] i3c: master: Report wakeup events for IBIs ---
commit 1c61ec6aa53c785eceea38530075c6fa6b7794f2
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Report wakeup events for IBIs

This commit records a wakeup event using pm_wakeup_event() when an
In-Band Interrupt (IBI) is queued for processing and the device has
wakeup enabled. It uses a 100 ms processing interval.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index cf3345265ac0b..9d56ca08569a1 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> @@ -3098,6 +3101,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
>  	if (!dev->ibi || !slot)
>  		return;
>  
> +	if (device_may_wakeup(&dev->dev->dev))
> +		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
> +

[Severity: High]
Does this introduce a regression where a race condition with device
unregistration could lead to a NULL pointer dereference or use-after-free
on dev->dev?

If an IBI fires and i3c_master_queue_ibi() executes concurrently with
device unregistration (for example, during driver module removal or
hot-unplug), i3c_master_unregister_i3c_devs() can free dev->dev and set it
to NULL:

drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
	...
		if (device_is_registered(&i3cdev->dev->dev))
			device_unregister(&i3cdev->dev->dev);
		else
			put_device(&i3cdev->dev->dev);
		i3cdev->dev = NULL;
	...
}

If i3c_master_queue_ibi() passes the lockless if (!dev->ibi) check just
before the unregistration runs, could the subsequent call to
device_may_wakeup() dereference a freed or NULL pointer?


>  	atomic_inc(&dev->ibi->pending_ibis);
>  	queue_work(dev->ibi->wq, &slot->work);
>  }

[Severity: High]
This is a pre-existing issue, but can accessing dev->ibi and its workqueue
here race with IBI teardown and cause a use-after-free?

If i3c_master_queue_ibi() passes the if (!dev->ibi) check, and concurrently
the driver is unbound, i3c_dev_free_ibi_locked() will destroy the workqueue
and free dev->ibi:

drivers/i3c/master.c:i3c_dev_free_ibi_locked() {
	...
	if (dev->ibi->wq) {
		destroy_workqueue(dev->ibi->wq);
		dev->ibi->wq = NULL;
	}

	kfree(dev->ibi);
	dev->ibi = NULL;
	...
}

Could the interrupt handler then continue execution and call atomic_inc()
and queue_work() using the recently freed dev->ibi struct and destroyed
workqueue?

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