Re: [PATCH] scsi: ufs: Remove redundant ret check in rpmb

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Incorrect device lifecycle management for `ufs_rpmb_dev` causes a reference cycle and Use-After-Free on device removal.
--

commit e8c5159f6cdc84b865b38447899724970aac1440
Author: Ao Sun <ao.sun@transsion.com>

scsi: ufs: Remove redundant ret check in rpmb

Removes an unnecessary check of the return value before calling
ufs_sec_submit() at the end of ufs_rpmb_route_frames(). All earlier error
paths return early, ensuring ret is zero at this point.

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index ffad049872b9a..55d0884dd4124 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -115,11 +115,9 @@ static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_l
>  		}
>  	}
>  
> -	if (!ret) {
> -		ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, false);
> -		if (ret)
> -			dev_err(dev, "Response read failed with ret=%d\n", ret);
> -	}
> +	ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, false);
> +	if (ret)
> +		dev_err(dev, "Response read failed with ret=%d\n", ret);
>  
>  	return ret;
>  }

[Severity: Critical]
This isn't a bug introduced by this patch, but there is a device lifecycle
issue causing a reference cycle and use-after-free on device removal.

The ufs_rpmb structure is allocated via devm_kzalloc() in ufs_rpmb_probe():

drivers/ufs/core/ufs-rpmb.c:ufs_rpmb_probe() {
    ...
    ufs_rpmb = devm_kzalloc(hba->dev, sizeof(*ufs_rpmb), GFP_KERNEL);
    ...
}

Because the child RPMB device (rdev) natively holds a reference to its
parent device (ufs_rpmb->dev), unregistering the child in the parent's
release callback creates an unbreakable cycle:

drivers/ufs/core/ufs-rpmb.c:ufs_rpmb_device_release() {
    ...
    rpmb_dev_unregister(ufs_rpmb->rdev);
    ...
}

Since ufs_rpmb_remove() unregisters the parent without explicitly
unregistering the child device:

drivers/ufs/core/ufs-rpmb.c:ufs_rpmb_remove() {
    ...
    device_unregister(&ufs_rpmb->dev);
    ...
}

The parent's release callback will never execute because the child still
holds a reference to it.

Because ufs_rpmb is allocated with devm_kzalloc(), its memory is forcefully
freed when the host device unbinds. Does this leave the driver core and
still-registered child device holding dangling pointers to freed memory,
guaranteeing a use-after-free during driver unbind or module unload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618062752.492-1-ao.sun@transsion.com?part=1