Re: [PATCH v3] scsi: ufs: core: Fix UFS RPMB device teardown order

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `ufs_rpmb` memory is freed while its child RPMB device (`rdev`) may still be in use by other subsystems (e.g., OP-TEE), leading to a Use-After-Free when routing frames.
--

--- Patch [1]: [PATCH v3] scsi: ufs: core: Fix UFS RPMB device teardown order ---
commit 61bb07aa128e1c613c9b0aaf64bf60f4bcd759c7
Author: Ao Sun <ao.sun@transsion.com>

scsi: ufs: core: Fix UFS RPMB device teardown order

This commit changes the UFS RPMB device teardown sequence to unregister the
child RPMB device before the parent device. It also moves the memory freeing
of the ufs_rpmb_dev struct to the parent device's release callback and
initializes the hba->rpmbs list earlier.

Link: https://lore.kernel.org/all/20260714064356.CF7101F000E9@smtp.kernel.org/

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index ffad049872b9a..cfd1f074d0043 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c

[ ... ]

> @@ -224,6 +222,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
>  	kfree(cid);
>  	list_for_each_entry_safe(it, tmp, &hba->rpmbs, node) {
>  		list_del(&it->node);
> +		rpmb_dev_unregister(it->rdev);
>  		device_unregister(&it->dev);

[Severity: High]
Could this teardown sequence result in a use-after-free if another
subsystem (like OP-TEE) has already acquired a reference to the RPMB device?

When rpmb_dev_unregister() is called, the driver core removes the child
from the hierarchy and drops its reference to the parent. The subsequent
device_unregister() on the parent drops its refcount to zero, triggering
ufs_rpmb_device_release() and freeing the ufs_rpmb object.

If a concurrent subsystem holds a reference to rdev via rpmb_dev_get(),
it might later call rpmb_route_frames(rdev, ...). This executes
rdev->descr.route_frames(rdev->dev.parent, ...).

Since dev.parent is now a dangling pointer to the freed parent device,
ufs_rpmb_route_frames() will access freed memory when fetching the driver
data.

>  	}
>  
> @@ -244,6 +243,7 @@ void ufs_rpmb_remove(struct ufs_hba *hba)
>  		/* Remove from list first */
>  		list_del(&ufs_rpmb->node);
>  		/* Unregister device */
> +		rpmb_dev_unregister(ufs_rpmb->rdev);
>  		device_unregister(&ufs_rpmb->dev);

[Severity: High]
Does this have the same use-after-free risk as the probe error path above?

If a concurrent subsystem retains a reference to the child device,
unregistering the devices in this order frees the parent object while the
child device might still route frames, leading to a use-after-free on the
parent device pointer.

>  	}

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