Re: [PATCH 1/2] scsi: ufs: rpmb: Decouple device lifecycle from devres to avoid UAF

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The patch fails to fully resolve the Use-After-Free (UAF) during concurrent RPMB accesses. The refcount-based lifecycle fix assumes the RPMB subsystem safely pins the parent device (`ufs_rpmb->dev`) for the lifetime of the RPMB device (`rdev`), but it drops this reference prematurely.
--

commit 21fd8683771415df1dde0fd462d6c27a0d117593
Author: Stanley Jhu <stanleyjhu@google.com>
Subject: scsi: ufs: rpmb: Decouple device lifecycle from devres to avoid UAF

This commit decouples the ufs_rpmb_dev lifecycle from devres, replacing it
with standard device reference counting via the release callback. It also
ensures the underlying SCSI device is pinned while the RPMB device exists,
and corrects subsystem teardown paths.

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index aa925cbb07e86..4f41d0b64d208 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
[ ... ]
> @@ -242,14 +257,13 @@ void ufs_rpmb_remove(struct ufs_hba *hba)
>  
>  	/* Remove all registered RPMB devices */
>  	list_for_each_entry_safe(ufs_rpmb, tmp, &hba->rpmbs, node) {
> -		dev_info(hba->dev, "Removing UFS RPMB region %d\n", ufs_rpmb->region_id);
> -		/* Remove from list first */
> -		list_del(&ufs_rpmb->node);
> -		/* Unregister device */
> +		list_del_init(&ufs_rpmb->node);
> +		if (ufs_rpmb->rdev) {
> +			rpmb_dev_unregister(ufs_rpmb->rdev);
> +			ufs_rpmb->rdev = NULL;
> +		}
>  		device_unregister(&ufs_rpmb->dev);
>  	}
> -
> -	dev_info(hba->dev, "All UFS RPMB devices unregistered\n");
>  }

[Severity: Critical]
This is a pre-existing issue, but does device_unregister() here prematurely
drop the refcount on ufs_rpmb->dev?

If an external client (like OP-TEE) has grabbed a reference to the RPMB
device via rpmb_dev_get(), it pins rdev. However, the implicit parent
reference held by rdev is dropped synchronously during rpmb_dev_unregister().
The subsequent call to device_unregister(&ufs_rpmb->dev) can drop
ufs_rpmb->dev's refcount to zero, freeing ufs_rpmb.

When OP-TEE later submits a request, it calls rpmb_route_frames() in
drivers/misc/rpmb-core.c with the dangling parent pointer:

	return rdev->descr.route_frames(rdev->dev.parent, req, req_len,
					rsp, rsp_len);

This dangling pointer is then passed into ufs_rpmb_route_frames() in
drivers/ufs/core/ufs-rpmb.c:

	struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);

Does this sequence still result in a use-after-free despite the new
refcount-based lifecycle?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904110017.3444852-1-stanleyjhu@google.com?part=1