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

On 28/07/2026 19:47, [email protected] wrote:
> 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`.

Already fixed by "i3c: master: Fix info leak and UAF in device unregister path"

	https://lore.kernel.org/all/[email protected]/

> 
> 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 <[email protected]>
> 
> 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?
>