[PATCH 0/2] scsi: ufs: rpmb: Fix device lifetime UAF and unregistered bus regression

[PATCH 0/2] scsi: ufs: rpmb: Fix device lifetime UAF and unregistered bus regression

[Stanley Jhu]

This series fixes two architectural defects in the UFS OP-TEE RPMB
driver: a use-after-free (UAF) during controller unbind or shutdown, and
an unregistered bus rejection by the driver core.

Patch 1: Decouple RPMB device lifecycle from devres
- Problem: struct ufs_rpmb_dev embeds a struct device but was allocated
  via devres tied to the parent controller. When the controller unbinds,
  devres prematurely frees the structure while external references
  remain, causing a UAF Oops on SLUB-poisoned memory
  (0x006b6b6b6b6b6b9c). Additionally, in-flight I/O lacks transport
  pinning to the underlying SCSI device.
- Solution: Transition to a reference-counted device lifecycle, pin the
  underlying SCSI device and host hierarchy during probe, and enforce
  proper subsystem unregistration and offline state validation.

Patch 2: Register and unregister ufs_rpmb_bus_type
- Problem: The driver assigns devices to ufs_rpmb_bus_type without
  registering the bus with the driver core, causing device_register()
  to fail with -EINVAL during probe.
- Solution: Register the bus during UFS core initialization and
  unregister it on exit, using IS_REACHABLE(CONFIG_RPMB) to provide clean
  fallback stubs and prevent modular build breakages.

Verification:
- QEMU ARM64 with virtual UFS 4.0 PCI controller:
  * Verified /sys/bus/ufs_rpmb registration and successful device probe.
  * Verified probe, unbind, and shutdown complete without leaks or Oops.
- checkpatch.pl: 0 errors, 0 warnings across all patches.

Stanley Jhu (2):
  scsi: ufs: rpmb: Decouple device lifecycle from devres to avoid UAF
  scsi: ufs: rpmb: Register and unregister ufs_rpmb_bus_type

 drivers/ufs/core/ufs-rpmb.c    | 96 ++++++++++++++++++++------------
 drivers/ufs/core/ufshcd-priv.h | 11 ++++-
 drivers/ufs/core/ufshcd.c      | 16 ++++++-
 3 files changed, 84 insertions(+), 39 deletions(-)

-- 
2.55.0.979.g7e5102b832-goog

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

[Stanley Jhu]

The UFS RPMB driver suffers from three architectural lifetime flaws:

1. Lifecycle mismatch:
   The driver allocates struct ufs_rpmb_dev via devres tied to the
   parent controller, even though it embeds a struct device. A struct
   device's lifetime must be governed by its own reference count.
   When the controller unbinds, devres prematurely frees the memory
   while references (such as sysfs nodes or userspace file descriptors)
   remain active, triggering a use-after-free (UAF) Oops:
     "Unable to handle kernel paging request at virtual address
      0x006b6b6b6b6b6b9c"

2. Unpinned transport hierarchy:
   The RPMB device relies on the underlying SCSI WLUN and host
   controller for command submission, but does not acquire a reference
   to the SCSI device. Unbinding the controller during in-flight I/O
   dereferences destroyed SCSI structures.

3. Inverted teardown and error handling:
   Subsystem registration is conflated with device memory lifecycle.
   The device release callback improperly attempts subsystem teardown,
   while driver removal and probe error paths unregister the driver
   core device without first unregistering from the RPMB subsystem.

To resolve these issues, align the driver with standard kernel device
model principles:

1. Reference-counted lifecycle:
   Decouple ufs_rpmb from devres. Manage its memory strictly through the
   embedded struct device's reference count, freeing it only in the
   device release callback.

2. Topological pinning:
   Pin the underlying SCSI device for the duration of the RPMB device's
   existence, ensuring the SCSI host hierarchy remains valid until all
   references to the RPMB device are dropped.

3. Symmetrical teardown and state validation:
   Unregister subsystem interfaces before driver core devices, correct
   error rollback paths, initialize fields before exposing the device
   to userspace, and reject requests if the underlying device goes
   offline.

Fixes: b06b8c421485 ("scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices")
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 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..4f41d0b64d20 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(sizeof(*ufs_rpmb), GFP_KERNEL);
 		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

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

[sashiko-bot]

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

[PATCH 2/2] scsi: ufs: rpmb: Register and unregister ufs_rpmb_bus_type

[Stanley Jhu]

The UFS RPMB driver assigns its devices to a custom ufs_rpmb_bus_type,
but never registers this bus with the driver core. As a result,
device_register() fails during probe because the driver core rejects
devices attached to unregistered buses with -EINVAL:
  "bus_add_device: cannot add device 'ufs_rpmb0' to unregistered bus
   'ufs_rpmb'"
  "ufshcd: Failed to register UFS RPMB device 0"

Resolve this by integrating the bus into the UFS core lifecycle:

1. Bus registration and lifecycle:
   Register ufs_rpmb_bus_type during ufshcd_core_init() before any
   device probe occurs, and unregister it during ufshcd_core_exit().
   Annotate the registration helper with __init to allow init memory
   reclamation.

2. Modular reachability:
   Guard bus helper prototypes with IS_REACHABLE(CONFIG_RPMB) and provide
   inline stubs, preventing link failures when UFS is built-in and RPMB
   is compiled as a module.

Fixes: b06b8c421485 ("scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices")
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufs-rpmb.c    | 10 ++++++++++
 drivers/ufs/core/ufshcd-priv.h | 11 ++++++++++-
 drivers/ufs/core/ufshcd.c      | 16 ++++++++++++++--
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index 4f41d0b64d20..cf8982824b14 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -29,6 +29,16 @@ static const struct bus_type ufs_rpmb_bus_type = {
 	.name = "ufs_rpmb",
 };
 
+int __init ufs_rpmb_bus_register(void)
+{
+	return bus_register(&ufs_rpmb_bus_type);
+}
+
+void ufs_rpmb_bus_unregister(void)
+{
+	bus_unregister(&ufs_rpmb_bus_type);
+}
+
 /* UFS RPMB device structure */
 struct ufs_rpmb_dev {
 	u8 region_id;
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index e55c2a02c1f5..415f9230b164 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -493,10 +493,19 @@ static inline u32 ufshcd_mcq_get_sq_head_slot(struct ufs_hw_queue *q)
 	return val / sizeof(struct utp_transfer_req_desc);
 }
 
-#if IS_ENABLED(CONFIG_RPMB)
+#if IS_REACHABLE(CONFIG_RPMB)
+int __init ufs_rpmb_bus_register(void);
+void ufs_rpmb_bus_unregister(void);
 int ufs_rpmb_probe(struct ufs_hba *hba);
 void ufs_rpmb_remove(struct ufs_hba *hba);
 #else
+static inline int ufs_rpmb_bus_register(void)
+{
+	return 0;
+}
+static inline void ufs_rpmb_bus_unregister(void)
+{
+}
 static inline int ufs_rpmb_probe(struct ufs_hba *hba)
 {
 	return 0;
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index a25b6a0cb5c6..4470e3d65c5c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -11584,16 +11584,28 @@ static int __init ufshcd_core_init(void)
 
 	ufs_debugfs_init();
 
+	ret = ufs_rpmb_bus_register();
+	if (ret)
+		goto err_debugfs;
+
 	ret = scsi_register_driver(&ufs_dev_wlun_template);
 	if (ret)
-		ufs_debugfs_exit();
+		goto err_rpmb_bus;
+
+	return 0;
+
+err_rpmb_bus:
+	ufs_rpmb_bus_unregister();
+err_debugfs:
+	ufs_debugfs_exit();
 	return ret;
 }
 
 static void __exit ufshcd_core_exit(void)
 {
-	ufs_debugfs_exit();
 	scsi_unregister_driver(&ufs_dev_wlun_template);
+	ufs_rpmb_bus_unregister();
+	ufs_debugfs_exit();
 }
 
 module_init(ufshcd_core_init);
-- 
2.55.0.979.g7e5102b832-goog