Re: [PATCH v3 2/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] This problem wasn't introduced by this patch, but the patch fails to resolve the pre-existing Use-After-Free during device removal. The device model's `device_del()` drops the parent reference, meaning `rdev->dev.parent` becomes a dangling pointer if a consumer holds a reference to `rdev`.
--

commit 19c2abff2bd57dbeee481738657ea4d9e1123a19
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: ufs: rpmb: Decouple device lifecycle from devres to avoid UAF

This commit changes the allocation of the UFS RPMB device from devres to
a reference-counted lifecycle. It also explicitly unregisters the RPMB
child device during host removal before unregistering the parent device
to prevent memory safety issues.

Cc: stable@vger.kernel.org

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index c7aade96c823e..33b9a4a1b120e 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c

[ ... ]

> @@ -252,14 +266,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);

[Severity: Critical]
This isn't a bug introduced by this patch, but does this sequence still
leave a use-after-free window during device removal?

When rpmb_dev_unregister() is called, it internally calls device_del() on
the child device, which immediately drops the reference to its parent device
(ufs_rpmb->dev).

If a consumer is actively holding a reference to the child device via
rpmb_dev_get(), the child device itself remains allocated. However, the
subsequent call to device_unregister() here drops the final reference to
the parent ufs_rpmb structure, causing it to be freed.

If the consumer then concurrently executes a request, the framework will
pass the now-dangling parent pointer to the route_frames callback:

drivers/ufs/core/ufs-rpmb.c:ufs_rpmb_route_frames() {
    struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
    ...
}

Could this dev_get_drvdata(dev) dereference the freed parent memory,
resulting in a use-after-free despite the new lifecycle management?

>  	}
> -
> -	dev_info(hba->dev, "All UFS RPMB devices unregistered\n");
>  }
>  
>  MODULE_LICENSE("GPL v2");

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