[PATCH v5 0/3] rpmb: Fix request serialisation and teardown races

[PATCH v5 0/3] rpmb: Fix request serialisation and teardown races

[Stanley Jhu]

This merges two series that were both last posted as v3:

  [PATCH v3] rpmb: core: Guard frame requests and teardown with mutex
  https://lore.kernel.org/all/20260910015515.1991789-1-stanleyjhu@google.com/

  [PATCH v3 0/2] scsi: ufs: rpmb: Fix bus registration and device lifecycle
  https://lore.kernel.org/all/20260910015503.1991119-1-stanleyjhu@google.com/

They turned out to be one problem. The UFS patches make RPMB registration
work again. Registering RPMB devices without the core fix triggers a
use-after-free on any unbind that races an in-flight request. Landing them
as two independent series would leave that window open in between.

The order is chosen so that no commit enables RPMB registration before the
lifetime handling and the serialisation are in place:

  1/3 fixes the generic core. It fixes the teardown race on eMMC today
      and carries a stable tag. It has no effect on UFS on current
      kernels, where nothing registers.
  2/3 fixes the UFS device lifetime. Still nothing registers.
  3/3 removes the never registered bus, which is what makes UFS RPMB
      devices appear again.

drivers/misc/rpmb-core.c and drivers/ufs/ are not in the same tree. 1/3
has no build or runtime dependency on the other two and can be taken on
its own; 2/3 and 3/3 must not land before it.

Verified on QEMU arm64 with KASAN, PROVE_LOCKING and SLUB_DEBUG_ON, against
a UFS device advertising four 4 MiB RPMB regions. Two kthreads on different
CPUs issue RPMB_GET_WRITE_COUNTER against the same region 20000 times each
and compare the nonce echoed back. A third thread holds an rpmb_dev
reference and keeps issuing requests across a host unbind. OP-TEE is the
only in-kernel consumer of rpmb_route_frames(), so an out-of-tree module
stands in for it.

  tree                  rpmb_dev  stolen responses  unbind
  --------------------  --------  ----------------  --------------------
  3/3 alone             4         16512 of 40000    KASAN use-after-free
  3/3 and 2/3, no 1/3   4         17030 of 40000    KASAN use-after-free
  all three             4         0 of 40000        clean

The intermediate points were booted and unbound as well. After 1/3 and
after 2/3 no rpmb_dev is registered, so neither test applies to them, and
neither point reports KASAN.

UFS RPMB is not a feature that never worked. bus_add_device() only began
rejecting devices on an unregistered bus in commit 36f35b8df697 ("driver
core: reject devices with unregistered buses") in v7.2-rc1. Reverting that
commit on the same base, with none of these patches applied, brings all
four rpmb_devs back. All four are still in /sys/class/rpmb after a host
unbind that leaves /sys/class/scsi_device empty. So 2/3 fixes a leak that
is live on v6.19 through v7.1. 3/3 restores what v7.2 disabled. Both
carry Cc: stable again. For a backport the three must be taken together
and in order: 3/3 without 1/3 re-enables registration with the core race
still open.

Upstream QEMU answers SECURITY PROTOCOL IN/OUT on the RPMB well known LU
with INVALID OPCODE. The three rows above therefore also needed a local
QEMU change that implements the authenticated frame state machine. I can
post that to qemu-devel separately, and send the test module to anyone
who wants to reproduce the numbers.

Changes since v4, mostly from Bean Huo's review:

  https://lore.kernel.org/all/20260913033633.3159296-1-stanleyjhu@google.com/

- v4 dropped Cc: stable from the UFS patches on the grounds that the
  feature has never worked on any released kernel. That is wrong, as
  explained above, and both tags are back
- shortened the patch 1 and 2 commit messages
- documented that the rpmb_dev mutex also serialises requests, not only
  guards against teardown
- patch 2 keeps list_del() and the two dev_info() calls, and drops a dead
  rdev check, so its diff is smaller
- corrected the JESD220F section references in patch 1
- picked up Bean Huo's Reviewed-by on all three patches

Changes since v3:
- merged the two series and reordered so registration is enabled last
- dropped the incorrect Tested: line from the core patch
- dropped Cc: stable from the UFS patches; the feature has never worked on
  any released kernel, so there is nothing to backport (wrong, retracted
  in v5 above)
- rewrote the commit messages around the measured results

Stanley Jhu (3):
  rpmb: core: Guard frame requests and teardown with mutex
  scsi: ufs: rpmb: Decouple device lifecycle from devres to avoid UAF
  scsi: ufs: rpmb: Drop the unregistered ufs_rpmb bus

 drivers/misc/rpmb-core.c    | 34 ++++++++++++---
 drivers/ufs/core/ufs-rpmb.c | 85 ++++++++++++++++++++-----------------
 include/linux/rpmb.h        |  5 +++
 3 files changed, 79 insertions(+), 45 deletions(-)

-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH v5 1/3] rpmb: core: Guard frame requests and teardown with mutex

[Stanley Jhu]

rpmb_route_frames() has no serialisation. That has two independent
consequences.

Concurrent requests on one rpmb_dev corrupt each other. Two kthreads
issuing RPMB_GET_WRITE_COUNTER against the same UFS RPMB region, 20000
iterations each:

  race complete: MISMATCH=17030 ERRORS=0

A mismatch is a response whose echoed nonce belongs to the other thread:
43% of requests returned somebody else's frame, with no transport error.
An authenticated RPMB operation is not a single command. Per JESD220F
12.4.2 a region processes one at a time. Per 12.4.7 any request other
than a result read overwrites the region's result register. The rpmb_dev
is the granularity the device itself assumes.

A request can also be in flight when the provider tears down.
rpmb_dev_unregister() calls device_del(), which drops the reference
device_add() took on the parent. The parent can then be freed while the
rpmb_dev is still alive and routable through rdev->dev.parent. Unbinding
a UFS host under a consumer that keeps issuing requests:

  BUG: KASAN: slab-use-after-free in ufs_rpmb_route_frames+0x328/0x420
  Read of size 8 at addr fff00000c833bce8 by task rpmb_hold/100
  Call trace:
   ufs_rpmb_route_frames+0x328/0x420
   rpmb_route_frames+0x64/0xd0
  Freed by task 1:
   kfree+0x2b8/0x5c4
   ufs_rpmb_device_release+0x3c/0x60
   device_release+0xa0/0x1fc
   device_unregister+0x20/0x38
   ufs_rpmb_remove+0x130/0x230
   ufshcd_remove+0x54/0x22c

The stable tag is for eMMC. mmc_route_rpmb_frames() packs the sequence
into one block request, so eMMC cannot interleave. The teardown window
is still open there. mmc_blk_remove() reaches rpmb_dev_unregister() well
before tearing down the queue that request goes to. That window is from
source reading, not reproduced.

Add a mutex and a dead flag to struct rpmb_dev. One lock held across the
whole request both serialises requests and excludes unregistration. The
kerneldoc is wrong as well: until rpmb_dev_unregister() runs, the child
holds a reference that keeps the parent's release callback from running,
so a provider cannot unregister from there. Both tests then report
MISMATCH=0 and a clean unbind.

Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---

Notes:
    Both measurements need all three patches applied: on current kernels no
    rpmb_dev registers on UFS, so this patch alone changes nothing there.

 drivers/misc/rpmb-core.c | 34 +++++++++++++++++++++++++++++-----
 include/linux/rpmb.h     |  5 +++++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index ecf14acf230a..bbc3c404ad6f 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -45,16 +45,28 @@ EXPORT_SYMBOL_GPL(rpmb_dev_put);
  * @rsp:	rpmb response frames
  * @rsp_len:	length of rpmb response frames in bytes
  *
+ * Context: Might sleep.
+ *
  * Returns: < 0 on failure
  */
 int rpmb_route_frames(struct rpmb_dev *rdev, u8 *req,
 		      unsigned int req_len, u8 *rsp, unsigned int rsp_len)
 {
-	if (!req || !req_len || !rsp || !rsp_len)
+	int ret;
+
+	if (!rdev || !req || !req_len || !rsp || !rsp_len)
 		return -EINVAL;
 
-	return rdev->descr.route_frames(rdev->dev.parent, req, req_len,
-					rsp, rsp_len);
+	mutex_lock(&rdev->lock);
+	if (rdev->dead) {
+		mutex_unlock(&rdev->lock);
+		return -ENODEV;
+	}
+
+	ret = rdev->descr.route_frames(rdev->dev.parent, req, req_len,
+				       rsp, rsp_len);
+	mutex_unlock(&rdev->lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(rpmb_route_frames);
 
@@ -62,6 +74,7 @@ static void rpmb_dev_release(struct device *dev)
 {
 	struct rpmb_dev *rdev = to_rpmb_dev(dev);
 
+	mutex_destroy(&rdev->lock);
 	ida_free(&rpmb_ida, rdev->id);
 	kfree(rdev->descr.dev_id);
 	kfree(rdev);
@@ -123,8 +136,9 @@ EXPORT_SYMBOL_GPL(rpmb_interface_unregister);
  * rpmb_dev_unregister() - unregister RPMB partition from the RPMB subsystem
  * @rdev: the rpmb device to unregister
  *
- * This function should be called from the release function of the
- * underlying device used when the RPMB device was registered.
+ * This function should be called from the remove or unbind callback of the
+ * underlying device used when the RPMB device was registered, never from
+ * a device release callback.
  *
  * Returns: < 0 on failure
  */
@@ -133,6 +147,14 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
 	if (!rdev)
 		return -EINVAL;
 
+	mutex_lock(&rdev->lock);
+	if (rdev->dead) {
+		mutex_unlock(&rdev->lock);
+		return 0;
+	}
+	rdev->dead = true;
+	mutex_unlock(&rdev->lock);
+
 	device_del(&rdev->dev);
 
 	rpmb_dev_put(rdev);
@@ -164,6 +186,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 	rdev = kzalloc_obj(*rdev);
 	if (!rdev)
 		return ERR_PTR(-ENOMEM);
+	mutex_init(&rdev->lock);
 	rdev->descr = *descr;
 	rdev->descr.dev_id = kmemdup(descr->dev_id, descr->dev_id_len,
 				     GFP_KERNEL);
@@ -194,6 +217,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 err_free_dev_id:
 	kfree(rdev->descr.dev_id);
 err_free_rdev:
+	mutex_destroy(&rdev->lock);
 	kfree(rdev);
 	return ERR_PTR(ret);
 }
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index ed3f8e431eff..65807f597735 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -7,6 +7,7 @@
 #define __RPMB_H__
 
 #include <linux/device.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
 
 /**
@@ -48,15 +49,19 @@ struct rpmb_descr {
  * struct rpmb_dev - device which can support RPMB partition
  *
  * @dev              : device
+ * @lock             : serialises requests and protects them against teardown
  * @id               : device_id
  * @list_node        : linked list node
  * @descr            : RPMB description
+ * @dead             : set to true when device is unregistered
  */
 struct rpmb_dev {
 	struct device dev;
+	struct mutex lock;	/* Serialises route_frames(), guards @dead */
 	int id;
 	struct list_head list_node;
 	struct rpmb_descr descr;
+	bool dead;
 };
 
 #define to_rpmb_dev(x)		container_of((x), struct rpmb_dev, dev)
-- 
2.55.0.1007.g17ff1f9808-goog

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

[Stanley Jhu]

struct ufs_rpmb_dev embeds a struct device but is allocated with
devm_kzalloc() against the host. That makes devres free it when the host
detaches, regardless of the device reference count. Probe takes no
reference on the RPMB well known LU either, so an in-flight request runs
on a freed scsi_device:

  BUG: KASAN: slab-use-after-free in scsi_execute_cmd+0x998/0xab0
  Read of size 8 at addr fff00000c82d4008 by task rpmb_hold/100
  Call trace:
   scsi_execute_cmd+0x998/0xab0
   ufs_sec_submit.isra.0+0x110/0x150
   ufs_rpmb_route_frames+0x148/0x460
   rpmb_route_frames+0x64/0xd0
  Freed by task 1:
   kfree+0x2b8/0x5c4
   scsi_device_dev_release+0x6b8/0xb7c
   __scsi_remove_device+0x1c8/0x318
   scsi_remove_host+0xc0/0x258
   ufshcd_remove+0x1c0/0x22c

The release callback cannot clean this up, because it never runs.
rpmb_dev_register() makes the rpmb_dev a child of ufs_rpmb->dev, so the
child holds a reference on the parent until rpmb_dev_unregister() runs.
The only caller of rpmb_dev_unregister() is the parent's own release
callback, ufs_rpmb_device_release(). With commit 36f35b8df697 ("driver
core: reject devices with unregistered buses") reverted so that
registration succeeds, all four rpmb_devs are still in /sys/class/rpmb
after the host is unbound and /sys/class/scsi_device is empty.

Tie the memory to the reference count instead:

- allocate with kzalloc_obj(), free in ufs_rpmb_device_release()
- pin the WLUN with scsi_device_get(), drop it in the same callback
- call rpmb_dev_unregister() from ufs_rpmb_remove() and the probe
  unwind, before device_unregister(), so the cycle is broken
- reject requests once the WLUN is offline

Since that commit nothing registers on UFS. On current kernels this
patch has no observable effect on its own; the trace above was taken
with registration restored.

Fixes: b06b8c421485 ("scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufs-rpmb.c | 80 +++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index 783ecfc7581d..a3e43902bf08 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -14,6 +14,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>
@@ -36,13 +37,14 @@ 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;
 	struct scsi_failure failure_defs[] = {
 		{
 			.sense = UNIT_ATTENTION,
@@ -61,6 +63,9 @@ static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t le
 	};
 	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]);
@@ -73,13 +78,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) {
@@ -87,8 +91,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;
@@ -121,7 +123,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;
@@ -132,7 +134,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;
@@ -140,7 +142,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);
 	}
@@ -150,23 +152,30 @@ 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 rpmb_descr descr = {
+		.type = RPMB_TYPE_UFS,
+		.route_frames = ufs_rpmb_route_frames,
+		.reliable_wr_count = hba->dev_info.rpmb_io_size,
+	};
+	struct scsi_device *sdev = hba->ufs_rpmb_wlun;
 	struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
 	u8 dev_id[UFS_RPMB_ID_LEN];
 	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;
 	}
@@ -177,25 +186,26 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 		return -EINVAL;
 	}
 
-	struct rpmb_descr descr = {
-		.type = RPMB_TYPE_UFS,
-		.route_frames = ufs_rpmb_route_frames,
-		.reliable_wr_count = hba->dev_info.rpmb_io_size,
-	};
-
 	for (region = 0; region < ARRAY_SIZE(hba->dev_info.rpmb_region_size); region++) {
 		cap = hba->dev_info.rpmb_region_size[region];
 		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;
+		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);
@@ -206,16 +216,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;
 		}
 
 		blake2b(NULL, 0, cid, strlen(cid), dev_id, UFS_RPMB_ID_LEN);
@@ -226,29 +234,30 @@ 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);
+		rpmb_dev_unregister(it->rdev);
 		device_unregister(&it->dev);
 	}
 
@@ -268,6 +277,7 @@ void ufs_rpmb_remove(struct ufs_hba *hba)
 		dev_info(hba->dev, "Removing UFS RPMB region %d\n", ufs_rpmb->region_id);
 		/* Remove from list first */
 		list_del(&ufs_rpmb->node);
+		rpmb_dev_unregister(ufs_rpmb->rdev);
 		/* Unregister device */
 		device_unregister(&ufs_rpmb->dev);
 	}
-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH v5 3/3] scsi: ufs: rpmb: Drop the unregistered ufs_rpmb bus

[Stanley Jhu]

ufs_rpmb_probe() assigns ufs_rpmb_bus_type to dev.bus, but that bus is
never passed to bus_register(). bus_add_device() used to return success
and leave such a device off the bus. UFS RPMB therefore registered from
v6.19 onwards, with /sys/bus/ufs_rpmb simply absent. Since commit
36f35b8df697 ("driver core: reject devices with unregistered buses") in
v7.2-rc1 it returns -EINVAL instead, and device_register() fails:

  bus_add_device: cannot add device 'ufs_rpmb0' to unregistered bus
  'ufs_rpmb'
  ufshcd 0000:00:02.0: Failed to register UFS RPMB device 0

ufs_rpmb_probe() unwinds on the first failure, so no region registers at
all and /sys/class/rpmb stays empty.

ufs_rpmb_bus_type declares no .match and no .probe, and no driver binds
to it. RPMB devices are exposed to consumers through /sys/class/rpmb/,
which rpmb_dev_register() already sets up. Drop the bus rather than
register it: device_register() works with dev.bus left NULL given a
parent and a release callback, both of which ufs_rpmb_probe() sets.

With the bus gone, on a device advertising four RPMB regions:

  ufshcd 0000:00:02.0: UFS RPMB region 0 registered (capacity=32)
  ufshcd 0000:00:02.0: UFS RPMB region 1 registered (capacity=32)
  ufshcd 0000:00:02.0: UFS RPMB region 2 registered (capacity=32)
  ufshcd 0000:00:02.0: UFS RPMB region 3 registered (capacity=32)

/sys/class/rpmb then holds rpmb0 to rpmb3, and unbinding the host
removes them.

Fixes: b06b8c421485 ("scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufs-rpmb.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index a3e43902bf08..ad3e8b054dc6 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -28,10 +28,6 @@
 #define UFS_RPMB_SEC_PROTOCOL		0xEC	/* JEDEC UFS application */
 #define UFS_RPMB_SEC_PROTOCOL_ID	0x01	/* JEDEC UFS RPMB protocol ID, CDB byte3 */
 
-static const struct bus_type ufs_rpmb_bus_type = {
-	.name = "ufs_rpmb",
-};
-
 /* UFS RPMB device structure */
 struct ufs_rpmb_dev {
 	u8 region_id;
@@ -206,7 +202,6 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 		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);
 
-- 
2.55.0.1007.g17ff1f9808-goog

Re: [PATCH v5 0/3] rpmb: Fix request serialisation and teardown races

[Stanley Jhu]

Hi Jens,

Gentle ping on this series. All three patches carry Bean's
Reviewed-by.

1/3 touches drivers/misc/rpmb-core.c and 2/3-3/3 depend on it. Are
you OK with Martin taking the whole series through the SCSI tree, or
would you prefer to pick up 1/3 yourself?

Thanks,
Stanley