From: Stanley Jhu <stanleyjhu@google.com>
To: Jens Wiklander <jens.wiklander@linaro.org>,
"Martin K . Petersen" <martin.petersen@oracle.com>,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Bart Van Assche <bvanassche@acm.org>,
Brian Kao <brian.kao@mediatek.com>,
Brian Kao <powenkao@google.com>,
Avri Altman <avri.altman@wdc.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"James E . J . Bottomley"
<James.Bottomley@HansenPartnership.com>,
Stanley Jhu <stanleyjhu@google.com>,
stable@vger.kernel.org
Subject: [PATCH v2 2/3] scsi: ufs: rpmb: Decouple device lifecycle from devres to avoid UAF
Date: Fri, 4 Sep 2026 23:53:06 +0800 [thread overview]
Message-ID: <20260904155307.150443-3-stanleyjhu@google.com> (raw)
In-Reply-To: <20260904155307.150443-1-stanleyjhu@google.com>
struct ufs_rpmb_dev embeds struct device, but is allocated using
devm_kzalloc() tied to the host controller (hba->dev).
When the host controller unbinds, devres automatically frees ufs_rpmb
regardless of active references to ufs_rpmb->dev (such as sysfs users or
child devices). Subsequent access to ufs_rpmb->dev triggers a
Use-After-Free (UAF) kernel Oops.
Fix this by decoupling ufs_rpmb from devres, managing its lifetime
strictly through the embedded device reference count, and pinning the
underlying SCSI device.
Fixes: b06b8c421485 ("scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices")
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
Cc: stable@vger.kernel.org
---
Changes in v2:
- Explicitly unregister rpmb device before unregistering ufs_rpmb dev.
drivers/ufs/core/ufs-rpmb.c | 86 +++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 36 deletions(-)
diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index aa925cbb07e8..fe96acdde278 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/device.h>
#include <linux/kernel.h>
+#include <linux/slab.h>
#include <linux/types.h>
#include <linux/rpmb.h>
#include <linux/string.h>
@@ -33,15 +34,19 @@ struct ufs_rpmb_dev {
u8 region_id;
struct device dev;
struct rpmb_dev *rdev;
- struct ufs_hba *hba;
+ struct scsi_device *sdev;
struct list_head node;
};
-static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t len, bool send)
+static int ufs_sec_submit(struct ufs_rpmb_dev *ufs_rpmb, u16 spsp,
+ void *buffer, size_t len, bool send)
{
- struct scsi_device *sdev = hba->ufs_rpmb_wlun;
+ struct scsi_device *sdev = ufs_rpmb->sdev;
u8 cdb[12] = { };
+ if (!sdev || !scsi_device_online(sdev))
+ return -ENODEV;
+
cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
cdb[1] = UFS_RPMB_SEC_PROTOCOL;
put_unaligned_be16(spsp, &cdb[2]);
@@ -53,13 +58,12 @@ static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t le
/* UFS RPMB route frames implementation */
static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_len, u8 *resp,
- unsigned int resp_len)
+ unsigned int resp_len)
{
struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
struct rpmb_frame *frm_out = (struct rpmb_frame *)req;
bool need_result_read = true;
u16 req_type, protocol_id;
- struct ufs_hba *hba;
int ret;
if (!ufs_rpmb) {
@@ -67,8 +71,6 @@ static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_l
return -ENODEV;
}
- hba = ufs_rpmb->hba;
-
/* req_resp is at the end of an RPMB frame. */
if (req_len < sizeof(*frm_out))
return -EINVAL;
@@ -101,7 +103,7 @@ static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_l
protocol_id = ufs_rpmb->region_id << 8 | UFS_RPMB_SEC_PROTOCOL_ID;
- ret = ufs_sec_submit(hba, protocol_id, req, req_len, true);
+ ret = ufs_sec_submit(ufs_rpmb, protocol_id, req, req_len, true);
if (ret) {
dev_err(dev, "Command failed with ret=%d\n", ret);
return ret;
@@ -112,7 +114,7 @@ static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_l
memset(frm_resp, 0, sizeof(*frm_resp));
put_unaligned_be16(RPMB_RESULT_READ, &frm_resp->req_resp);
- ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, true);
+ ret = ufs_sec_submit(ufs_rpmb, protocol_id, resp, resp_len, true);
if (ret) {
dev_err(dev, "Result read request failed with ret=%d\n", ret);
return ret;
@@ -120,7 +122,7 @@ static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_l
}
if (!ret) {
- ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, false);
+ ret = ufs_sec_submit(ufs_rpmb, protocol_id, resp, resp_len, false);
if (ret)
dev_err(dev, "Response read failed with ret=%d\n", ret);
}
@@ -130,22 +132,24 @@ static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_l
static void ufs_rpmb_device_release(struct device *dev)
{
- struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
+ struct ufs_rpmb_dev *ufs_rpmb = container_of(dev, struct ufs_rpmb_dev, dev);
- rpmb_dev_unregister(ufs_rpmb->rdev);
+ scsi_device_put(ufs_rpmb->sdev);
+ kfree(ufs_rpmb);
}
/* UFS RPMB device registration */
int ufs_rpmb_probe(struct ufs_hba *hba)
{
+ struct scsi_device *sdev = hba->ufs_rpmb_wlun;
struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
struct rpmb_dev *rdev;
- char *cid = NULL;
+ char *cid;
int region;
u32 cap;
int ret;
- if (!hba->ufs_rpmb_wlun || hba->dev_info.b_advanced_rpmb_en) {
+ if (!sdev || hba->dev_info.b_advanced_rpmb_en) {
dev_info(hba->dev, "Skip OP-TEE RPMB registration\n");
return -ENODEV;
}
@@ -167,14 +171,23 @@ 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;
}
- ufs_rpmb->hba = hba;
- ufs_rpmb->dev.parent = &hba->ufs_rpmb_wlun->sdev_gendev;
+ INIT_LIST_HEAD(&ufs_rpmb->node);
+
+ ret = scsi_device_get(sdev);
+ if (ret) {
+ kfree(ufs_rpmb);
+ goto err_out;
+ }
+
+ ufs_rpmb->sdev = sdev;
+ ufs_rpmb->region_id = region;
+ ufs_rpmb->dev.parent = &sdev->sdev_gendev;
ufs_rpmb->dev.bus = &ufs_rpmb_bus_type;
ufs_rpmb->dev.release = ufs_rpmb_device_release;
dev_set_name(&ufs_rpmb->dev, "ufs_rpmb%d", region);
@@ -185,16 +198,14 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
ret = device_register(&ufs_rpmb->dev);
if (ret) {
dev_err(hba->dev, "Failed to register UFS RPMB device %d\n", region);
- put_device(&ufs_rpmb->dev);
- goto err_out;
+ goto err_put;
}
/* Create unique ID by appending region number to device_id */
cid = kasprintf(GFP_KERNEL, "%s-R%d", hba->dev_info.device_id, region);
if (!cid) {
- device_unregister(&ufs_rpmb->dev);
ret = -ENOMEM;
- goto err_out;
+ goto err_unreg;
}
descr.dev_id = cid;
@@ -203,29 +214,33 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
/* Register RPMB device */
rdev = rpmb_dev_register(&ufs_rpmb->dev, &descr);
+ kfree(cid);
if (IS_ERR(rdev)) {
dev_err(hba->dev, "Failed to register UFS RPMB device.\n");
- device_unregister(&ufs_rpmb->dev);
ret = PTR_ERR(rdev);
- goto err_out;
+ goto err_unreg;
}
- kfree(cid);
- cid = NULL;
-
ufs_rpmb->rdev = rdev;
- ufs_rpmb->region_id = region;
-
list_add_tail(&ufs_rpmb->node, &hba->rpmbs);
dev_info(hba->dev, "UFS RPMB region %d registered (capacity=%u)\n", region, cap);
}
return 0;
+
+err_unreg:
+ device_unregister(&ufs_rpmb->dev);
+ goto err_out;
+err_put:
+ put_device(&ufs_rpmb->dev);
err_out:
- kfree(cid);
list_for_each_entry_safe(it, tmp, &hba->rpmbs, node) {
- list_del(&it->node);
+ list_del_init(&it->node);
+ if (it->rdev) {
+ rpmb_dev_unregister(it->rdev);
+ it->rdev = NULL;
+ }
device_unregister(&it->dev);
}
@@ -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");
}
MODULE_LICENSE("GPL v2");
--
2.55.0.979.g7e5102b832-goog
next prev parent reply other threads:[~2026-09-04 15:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 15:53 [PATCH v2 0/3] Fix RPMB cross-subsystem UAF and UFS OP-TEE RPMB lifecycle Stanley Jhu
2026-09-04 15:53 ` [PATCH v2 1/3] rpmb: core: Pin parent device and guard requests with rwsem Stanley Jhu
2026-09-04 15:53 ` Stanley Jhu [this message]
2026-09-04 15:53 ` [PATCH v2 3/3] scsi: ufs: rpmb: Register and unregister ufs_rpmb_bus_type Stanley Jhu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260904155307.150443-3-stanleyjhu@google.com \
--to=stanleyjhu@google.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=avri.altman@wdc.com \
--cc=brian.kao@mediatek.com \
--cc=bvanassche@acm.org \
--cc=gregkh@linuxfoundation.org \
--cc=jens.wiklander@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=powenkao@google.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox