Re: [PATCH v1 2/2] ufs: rpmb: use a fixed-length RPMB dev_id

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `ufs_rpmb->dev` device release function is structurally prevented from running, leaking RPMB child devices and their associated memory.
- [High] The `struct ufs_rpmb_dev` embeds a refcounted `struct device` but is allocated via `devm_kzalloc()`, leading to a use-after-free.
--

--- Patch [2]: [PATCH v1 2/2] ufs: rpmb: use a fixed-length RPMB dev_id ---
commit b0359490487ec8ad9999a553b0e7ff80b787f32c
Author: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>

ufs: rpmb: use a fixed-length RPMB dev_id

This commit modifies the UFS RPMB driver to generate a fixed 16-byte
device ID by hashing the variable-length UFS device ID and region string
using blake2s. This aligns the UFS device ID length with the eMMC CID
format, allowing OP-TEE's key derivation to work without modification.

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index d0c7ea7a36f49..b800871269bb3 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
[ ... ]
> @@ -154,6 +156,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
>  {
>  	struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;

[Severity: High]
This is a pre-existing issue, but does embedding a refcounted struct device
in a devres-managed allocation lead to a use-after-free?

The ufs_rpmb structure is allocated via devm_kzalloc() in the probe loop.
Its lifecycle is tied to the host controller's devres, which frees the
memory when the controller unbinds.

However, the embedded struct device is refcounted independently. If any
references (such as sysfs files) remain when devres frees the structure,
the final reference drop will attempt to access the already-freed struct
device to call ufs_rpmb_device_release(), leading to a use-after-free.

>  	struct rpmb_dev *rdev;
> +	char *dev_id = NULL;
>  	char *cid = NULL;
[ ... ]
> @@ -240,6 +254,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
>  	return 0;
>  err_out:
>  	kfree(cid);
> +	kfree(dev_id);
>  	list_for_each_entry_safe(it, tmp, &hba->rpmbs, node) {
>  		list_del(&it->node);
>  		device_unregister(&it->dev);

[Severity: High]
This is a pre-existing issue, but does this error path permanently leak
RPMB child devices and their associated memory?

The rpmb_dev_register() call creates a child device (rdev) with
ufs_rpmb->dev as its parent, which automatically takes a reference on the
parent.

When device_unregister(&it->dev) is called here, the parent's refcount
does not drop to zero because the child still holds a reference. This
prevents ufs_rpmb_device_release() from ever running, meaning
rpmb_dev_unregister() is never called for the child, forming an
unbreakable circular dependency.

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716083728.2226422-1-jorge.ramirez@oss.qualcomm.com?part=2