Re: [PATCH V4 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices

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

Pre-existing issues:
- [High] Workqueue and rescue thread leak in error path of i3c_dev_request_ibi_locked().
--

--- Patch [11]: [PATCH V4 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices ---
commit cc9c665c421a1d987949a06269af6968afd5a89d
Author: Adrian Hunter <[email protected]>
Subject: i3c: master: Reject IBI requests from non-IBI-capable devices

This commit adds a capability check before allowing devices to set up IBI
requests. It fails with -EOPNOTSUPP if the device does not advertise IBI
support, ensuring consistency with other IBI features.

> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 5e6df6de02834..f1ba363b22a17 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -204,12 +204,14 @@ int i3c_device_request_ibi(struct i3c_device *dev,
>  		return ret;
>  
>  	i3c_bus_normaluse_lock(dev->bus);
> -	if (dev->desc) {
> +	if (!dev->desc) {
> +		ret = -ENOENT;
> +	} else if (!(dev->desc->info.bcr & I3C_BCR_IBI_REQ_CAP)) {
> +		ret = -EOPNOTSUPP;
> +	} else {
>  		mutex_lock(&dev->desc->ibi_lock);
>  		ret = i3c_dev_request_ibi_locked(dev->desc, req);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
i3c_dev_request_ibi_locked() leak the workqueue?

When master->ops->request_ibi() fails, the previously allocated workqueue
appears to be orphaned:

drivers/i3c/master.c:i3c_dev_request_ibi_locked() {
    ...
	ibi->wq = alloc_ordered_workqueue(dev_name(...), WQ_MEM_RECLAIM);
	if (!ibi->wq) {
        ...
	}
    ...
	ret = master->ops->request_ibi(dev, req);
	if (ret) {
		kfree(ibi);
		dev->ibi = NULL;
	}
    ...
}

Can this leave the wq and its associated rescue worker thread permanently
leaked since destroy_workqueue() is never called before kfree(ibi)?

>  		mutex_unlock(&dev->desc->ibi_lock);
> -	} else {
> -		ret = -ENOENT;
>  	}
>  	i3c_bus_normaluse_unlock(dev->bus);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11