drivers/ufs/core/ufs-rpmb.c | 8 ++++---- drivers/ufs/core/ufshcd.c | 1 + 2 files changed, 5 insertions(+), 4 deletions(-)
From: Ao Sun <ao.sun@transsion.com>
The child RPMB device holds a reference to its parent, so the parent's
release callback cannot be invoked if the child device is still registered.
Remove the rpmb_dev_unregister() from the parent release handler, and
unregister the child RPMB device ahead of the parent device in the remove
path.
Memory for struct ufs_rpmb_dev is allocated via kzalloc_obj(), and free it
from the device release callback, following the same pattern as MMC RPMB.
Initialize the hba->rpmbs list in ufshcd_alloc_host() to prevent NULL
pointer dereference in the device teardown path if ufs_rpmb_probe()
fails.
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260714064356.CF7101F000E9@smtp.kernel.org/
Signed-off-by: Jiazi Li <jiazi.li@transsion.com>
Signed-off-by: Ao Sun <ao.sun@transsion.com>
---
Changes in v3:
- switch devm_kzalloc() to kzalloc_obj()
- init rpmbs list in ufshcd
---
Changes in v2:
- drop the release callback
- init rpmbs list early
---
drivers/ufs/core/ufs-rpmb.c | 8 ++++----
drivers/ufs/core/ufshcd.c | 1 +
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index ffad049872b9..cfd1f074d004 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -128,7 +128,7 @@ static void ufs_rpmb_device_release(struct device *dev)
{
struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
- rpmb_dev_unregister(ufs_rpmb->rdev);
+ kfree(ufs_rpmb);
}
/* UFS RPMB device registration */
@@ -152,8 +152,6 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
return -EINVAL;
}
- INIT_LIST_HEAD(&hba->rpmbs);
-
struct rpmb_descr descr = {
.type = RPMB_TYPE_UFS,
.route_frames = ufs_rpmb_route_frames,
@@ -165,7 +163,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
if (!cap)
continue;
- ufs_rpmb = devm_kzalloc(hba->dev, sizeof(*ufs_rpmb), GFP_KERNEL);
+ ufs_rpmb = kzalloc_obj(*ufs_rpmb);
if (!ufs_rpmb) {
ret = -ENOMEM;
goto err_out;
@@ -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);
}
@@ -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);
}
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d3044a3089b5..60227069db06 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10988,6 +10988,7 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
hba->nop_out_timeout = NOP_OUT_TIMEOUT;
ufshcd_set_sg_entry_size(hba, sizeof(struct ufshcd_sg_entry));
INIT_LIST_HEAD(&hba->clk_list_head);
+ INIT_LIST_HEAD(&hba->rpmbs);
spin_lock_init(&hba->outstanding_lock);
*hba_handle = hba;
--
2.34.1
On Tue, 2026-07-21 at 08:41 +0000, Ao Sun wrote:
> From: Ao Sun <ao.sun@transsion.com>
>
> The child RPMB device holds a reference to its parent, so the parent's
> release callback cannot be invoked if the child device is still registered.
> Remove the rpmb_dev_unregister() from the parent release handler, and
> unregister the child RPMB device ahead of the parent device in the remove
> path.
>
> Memory for struct ufs_rpmb_dev is allocated via kzalloc_obj(), and free it
> from the device release callback, following the same pattern as MMC RPMB.
>
> Initialize the hba->rpmbs list in ufshcd_alloc_host() to prevent NULL
> pointer dereference in the device teardown path if ufs_rpmb_probe()
> fails.
>
> Reported-by: sashiko-bot@kernel.org
> Closes:
> https://lore.kernel.org/all/20260714064356.CF7101F000E9@smtp.kernel.org/
> Signed-off-by: Jiazi Li <jiazi.li@transsion.com>
> Signed-off-by: Ao Sun <ao.sun@transsion.com>
> ---
> Changes in v3:
> - switch devm_kzalloc() to kzalloc_obj()
> - init rpmbs list in ufshcd
> ---
> Changes in v2:
> - drop the release callback
> - init rpmbs list early
> ---
> drivers/ufs/core/ufs-rpmb.c | 8 ++++----
> drivers/ufs/core/ufshcd.c | 1 +
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index ffad049872b9..cfd1f074d004 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -128,7 +128,7 @@ static void ufs_rpmb_device_release(struct device *dev)
> {
> struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
>
> - rpmb_dev_unregister(ufs_rpmb->rdev);
> + kfree(ufs_rpmb);
> }
>
> /* UFS RPMB device registration */
> @@ -152,8 +152,6 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
> return -EINVAL;
> }
>
> - INIT_LIST_HEAD(&hba->rpmbs);
> -
> struct rpmb_descr descr = {
> .type = RPMB_TYPE_UFS,
> .route_frames = ufs_rpmb_route_frames,
> @@ -165,7 +163,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
> if (!cap)
> continue;
>
> - ufs_rpmb = devm_kzalloc(hba->dev, sizeof(*ufs_rpmb),
> GFP_KERNEL);
> + ufs_rpmb = kzalloc_obj(*ufs_rpmb);
> if (!ufs_rpmb) {
> ret = -ENOMEM;
> goto err_out;
> @@ -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);
> }
>
> @@ -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);
> }
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index d3044a3089b5..60227069db06 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -10988,6 +10988,7 @@ int ufshcd_alloc_host(struct device *dev, struct
> ufs_hba **hba_handle)
> hba->nop_out_timeout = NOP_OUT_TIMEOUT;
> ufshcd_set_sg_entry_size(hba, sizeof(struct ufshcd_sg_entry));
> INIT_LIST_HEAD(&hba->clk_list_head);
> + INIT_LIST_HEAD(&hba->rpmbs);
> spin_lock_init(&hba->outstanding_lock);
>
> *hba_handle = hba;
Hi Ao,
Thanks for the patch. I reviewed the whole RPMB core and OPTEE call sequencey,
the issue is real,
The issue is the lifetime of ufs_rpmb / ufs_rpmb->dev versus the rpmb device
rdev when a consumer holds a reference of rdev.
but I don't think we can take this patch. moving rpmb_dev_unregister() into the
remove path and freeing ufs_rpmb eagerly, then make use-after-free.
OP-TEE caches the rpmb device: on the RPMB probe path: optee->rpmb_dev =
rpmb_dev_find_device(), whch taking its own reference, and reuses that handle
for every subsequent frame in handle_rpc_func_rpmb_frames(). It only drops the
reference in secure-world rescan or at optee teardown. And rpmb_class_intf has
only add_dev, but no remove_dev, so nothing tells OP-TEE core when we unregister
the device.
I think the correct fix belongs in rpmb-core: a revocation barrier so that once
rpmb_dev_unregister() returns, route_frames() is guaranteed never to run
again, for example:
route_frames()
{
down_read(&rdev->route_lock); //block
if (rdev->no_route)
ret = -ENODEV;
else
ret = rdev->descr.route_frames(rdev->dev.parent, ...);
up_read(&rdev->route_lock);
return ret;
}
int rpmb_dev_unregister(struct rpmb_dev *rdev)
{
down_write(&rdev->route_lock); //drain in-flight, block future
rdev->no_route = true;
up_write(&rdev->route_lock);
device_del(&rdev->dev);
rpmb_dev_put(rdev);
return 0;
}
@jens, how do you think? I'm happy to help with the rpmb-core piece if that's
useful.
the INIT_LIST_HEAD() move is a separate patch (NULL deref on the early-return
probe paths, since hba->rpmbs is left zeroed). Please split it into new patch
with a Fixes tag.
Kind regards,
Bean
© 2016 - 2026 Red Hat, Inc.