Re: [PATCH V3 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] The workqueue `ibi->wq` is leaked on the error path if `master->ops->request_ibi(dev, req)` fails.
--

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

i3c: master: Reject IBI requests from non-IBI-capable devices

This commit updates i3c_device_request_ibi() to verify that a device
advertises IBI support before attempting to set up IBI handling. If the
device lacks the I3C_BCR_IBI_REQ_CAP capability, it now returns early
with -EOPNOTSUPP.

> 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 is a pre-existing issue, but since this patch interacts closely with IBI
request error handling, I noticed a potential resource leak in the underlying
i3c_dev_request_ibi_locked() function.

Does this code leak the newly allocated workqueue if the request fails?

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

Should destroy_workqueue(ibi->wq) be called before kfree(ibi) on this error
path to prevent leaking the workqueue if a driver repeatedly fails to request
an 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

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c