Skip to contentSkip to conversation

ufs: rpmb: make RPMB usable with OP-TEE key derivation

PATCH Applied Jorge Ramirez-Ortiz 3 participants 6 messages Aug 31 – Sep 17

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v5 cover ufs: rpmb: make RPMB usable with OP-TEE key derivation

This series makes UFS RPMB work out of the box with an OP-TEE that implements the standard eMMC RPMB key-derivation flow, without requiring any fundamental changes on the OP-TEE side.

RPMB provides an authenticated, replay-protected storage area whose security relies on a secret authentication key. In our setup that key is never exposed to the kernel: OP-TEE derives it in the secure world from its hardware-unique key and a device identifier (dev_id) that the RPMB core hands down. OP-TEE's implementation targets eMMC, where dev_id is the 16-byte eMMC CID, and both the fixed length and the raw-CID layout are baked into its key derivation.

Two things stand in the way of reusing that same, unmodified OP-TEE flow for UFS RPMB:

1. On a cold boot the very first frame sent to the RPMB well-known LU comes back with a power-on UNIT ATTENTION (ASC 0x29), which the SCSI core reports rather than retries. RPMB has no earlier guaranteed access that could clear the condition first, so RPMB fails on every power cycle. Patch 1 asks the SCSI core to retry the power-on UNIT ATTENTION on the RPMB WLUN.

2. The UFS RPMB id is "<device_id>-R<region>", which is variable length and longer than 16 bytes. Passing it verbatim would tie the derived key to a length OP-TEE does not expect and diverge from the fixed eMMC CID ABI. Patch 2 hashes it into a fixed 16-byte dev_id with blake2b, keeping the key stable and unique per region while matching the eMMC CID layout OP-TEE relies on. The hash algorithm and input string are thus part of the key-derivation ABI and must stay stable.

With both patches, UFS RPMB is functional from the first access after a cold boot and derives keys through the existing eMMC-style OP-TEE flow, (requires minimal OP-TEE changes pending on the CID proposal done here).

Tested on IQ-9075 with Open Firmware [1], pending OP-TEE changes [1]https://ldts.github.io/qcom-buildroot

Dependencies:

U-boot:
https://lore.kernel.org/u-boot/[email protected]/T/#mc423eb4dcf8a15849077029e4f7c1913bb7d8873

OP-TEE:
https://github.com/OP-TEE/optee_os/pull/7881

v5:
* added Reviewed-by tags from Bean Huo and Stanley Jhu; no code change.

v4:
* ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN: open-code the power-on ASC 0x29 (with a naming comment) as the rest of the SCSI tree does, instead of a UFS_RPMB_ASC_POWER_ON define; add a UFS_RPMB_UA_RETRIES define for the retry count; reworded the commit message.
* ufs: rpmb: use a fixed-length RPMB dev_id: reworded the commit message; no functional change.

v3:
* ufs: rpmb: use a fixed-length RPMB dev_id: hash into a stack buffer instead of a kzalloc'd one; rpmb_dev_register() copies dev_id, so the heap allocation and its cleanup were unnecessary.

v2:
* ufs: rpmb: replace blake2s with blake2b so that the same support can be added to u-boot (CRYPTO_LIB_BLAKE2B)
* added links to U-boot and OP-TEE changes.

v1:
* ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN:
- fix using uses SCMD_FAILURE_ASC_ANY to retry any Unit Attention
- fix unused variable
* ufs: rpmb: use a fixed-length RPMB dev_id
- fix selecting a non-existent Kconfig symbol

Jorge Ramirez-Ortiz (2):
ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN
ufs: rpmb: use a fixed-length RPMB dev_id

 drivers/ufs/Kconfig         |  1 +
 drivers/ufs/core/ufs-rpmb.c | 29 ++++++++++++++++++++++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)
Signature · 1 line
2.54.0
#1 lore raw

PATCH v5 1/2 ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN

After a power cycle, the first command sent to any UFS logical unit completes with CHECK CONDITION reporting a power-on UNIT ATTENTION. The SCSI core surfaces this condition to the caller rather than retrying it.

For the RPMB well-known LU the first command after boot is the first RPMB frame, and RPMB has no earlier, guaranteed access that could clear the condition beforehand. The power-on UNIT ATTENTION therefore reaches RPMB and fails that first frame, breaking RPMB on every cold boot.

The RPMB WLUN needs the power-on UNIT ATTENTION to be retried so that RPMB works from the very first access after a power cycle.

---
 drivers/ufs/core/ufs-rpmb.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
drivers/ufs/core/ufs-rpmb.c +19 -1
index ffad049872b9..79e6cd2b20a0 100644
@@ -21,6 +21,7 @@
 #include <linux/unaligned.h>
 #include "ufshcd-priv.h"
 
+#define UFS_RPMB_UA_RETRIES		3	/* Retries for the power-on UNIT ATTENTION */
 #define UFS_RPMB_SEC_PROTOCOL		0xEC	/* JEDEC UFS application */
 #define UFS_RPMB_SEC_PROTOCOL_ID	0x01	/* JEDEC UFS RPMB protocol ID, CDB byte3 */
 
@@ -40,6 +41,22 @@ struct ufs_rpmb_dev {
 static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t len, bool send)
 {
 	struct scsi_device *sdev = hba->ufs_rpmb_wlun;
+	struct scsi_failure failure_defs[] = {
+		{
+			.sense = UNIT_ATTENTION,
+			.asc = 0x29,	/* power on, reset, or bus device reset occurred */
+			.ascq = SCMD_FAILURE_ASCQ_ANY,
+			.allowed = UFS_RPMB_UA_RETRIES,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		{}
+	};
+	struct scsi_failures failures = {
+		.failure_definitions = failure_defs,
+	};
+	const struct scsi_exec_args exec_args = {
+		.failures = &failures,
+	};
 	u8 cdb[12] = { };
 
 	cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
@@ -48,7 +65,8 @@ static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t le
 	put_unaligned_be32(len, &cdb[6]);
 
 	return scsi_execute_cmd(sdev, cdb, send ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
-				buffer, len, /*timeout=*/30 * HZ, 0, NULL);
+				buffer, len, /*timeout=*/30 * HZ, /*retries=*/0,
+				&exec_args);
 }
 
 /* UFS RPMB route frames implementation */
Signature · 1 line
2.54.0
#2 lore raw

PATCH v5 2/2 ufs: rpmb: use a fixed-length RPMB dev_id

In reply to Jorge Ramirez-Ortiz

The RPMB authentication key is derived from the dev_id handed to the RPMB subsystem. OP-TEE implements the eMMC RPMB flow, where the dev_id is the eMMC CID: a fixed 16-byte value the key derivation depends on.

The UFS RPMB id is "<device_id>-R<region>", which is variable length and longer than 16 bytes. Handing it to the RPMB subsystem as-is would tie the derived key to a length OP-TEE does not expect and diverge from the fixed-CID eMMC ABI, forcing OP-TEE to be taught about variable-length UFS ids.

A fixed 16-byte dev_id is needed so the derived key stays stable and unique per region while matching the eMMC CID layout OP-TEE relies on, keeping the key-derivation ABI identical with no OP-TEE change. The reduction to a fixed 16 bytes must also be reproducible by the bootloaders (such as U-Boot) that derive the same dev_id.

---
 drivers/ufs/Kconfig         | 1 +
 drivers/ufs/core/ufs-rpmb.c | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/ufs/Kconfig +1 -0
index f662e7ce71f1..b62c00e7ff06 100644
@@ -7,6 +7,7 @@ menuconfig SCSI_UFSHCD
 	tristate "Universal Flash Storage Controller"
 	depends on SCSI && SCSI_DMA
 	depends on RPMB || !RPMB
+	select CRYPTO_LIB_BLAKE2B if RPMB
 	select PM_DEVFREQ
 	select DEVFREQ_GOV_SIMPLE_ONDEMAND
 	select NLS
drivers/ufs/core/ufs-rpmb.c +7 -2
index 79e6cd2b20a0..cb3ac23fa01a 100644
@@ -10,6 +10,7 @@
  *	Can Guo <[email protected]>
  */
 
+#include <crypto/blake2b.h>
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -21,6 +22,7 @@
 #include <linux/unaligned.h>
 #include "ufshcd-priv.h"
 
+#define UFS_RPMB_ID_LEN			16	/* Match eMMC CID Length */
 #define UFS_RPMB_UA_RETRIES		3	/* Retries for the power-on UNIT ATTENTION */
 #define UFS_RPMB_SEC_PROTOCOL		0xEC	/* JEDEC UFS application */
 #define UFS_RPMB_SEC_PROTOCOL_ID	0x01	/* JEDEC UFS RPMB protocol ID, CDB byte3 */
@@ -153,6 +155,7 @@ static void ufs_rpmb_device_release(struct device *dev)
 int ufs_rpmb_probe(struct ufs_hba *hba)
 {
 	struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
+	u8 dev_id[UFS_RPMB_ID_LEN];
 	struct rpmb_dev *rdev;
 	char *cid = NULL;
 	int region;
@@ -213,8 +216,10 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 			goto err_out;
 		}
 
-		descr.dev_id = cid;
-		descr.dev_id_len = strlen(cid);
+		blake2b(NULL, 0, cid, strlen(cid), dev_id, UFS_RPMB_ID_LEN);
+
+		descr.dev_id = dev_id;
+		descr.dev_id_len = UFS_RPMB_ID_LEN;
 		descr.capacity = cap;
 
 		/* Register RPMB device */
Signature · 1 line
2.54.0
#3 lore raw

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

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

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

Pre-existing issues:
- [High] Allocating a structure embedding a reference-counted struct device with devm_kzalloc() causes a Use-After-Free due to lifecycle mismatch.
- [High] Circular reference between ufs_rpmb->dev and rdev prevents device cleanup, leading to permanent resource leaks of both device structures.
- [High] Initialization race: RPMB device is exposed to the subsystem and notifiers before region_id is initialized.
- [High] The ufs_rpmb_bus_type is never registered with the driver core, causing device_register() to deterministically fail. --

commit a3eaa4c56fa0d1803a39f46b9f298247acd5bdfd
Author: Jorge Ramirez-Ortiz <[email protected]>

ufs: rpmb: use a fixed-length RPMB dev_id

The RPMB authentication key is derived from the dev_id handed to the RPMB subsystem. OP-TEE implements the eMMC RPMB flow, where the dev_id is the eMMC CID: a fixed 16-byte value the key derivation depends on.

[ ... ]

drivers/ufs/core/ufs-rpmb.c line 28

Quoted 12 lines
drivers/ufs/core/ufs-rpmb.c +1 -0
index 00ba4f3b842bf..783ecfc7581d7 100644
@@ -21,6 +22,7 @@
 #include <linux/unaligned.h>
 #include "ufshcd-priv.h"
 
+#define UFS_RPMB_ID_LEN			16	/* Match eMMC CID Length */
 #define UFS_RPMB_UA_RETRIES		3	/* Retries for the power-on UNIT ATTENTION */
 #define UFS_RPMB_SEC_PROTOCOL		0xEC	/* JEDEC UFS application */
 #define UFS_RPMB_SEC_PROTOCOL_ID	0x01	/* JEDEC UFS RPMB protocol ID, CDB byte3 */

[Severity: High]
This is a pre-existing issue, but the ufs_rpmb_bus_type defined nearby is completely missing a corresponding bus_register() call anywhere in the codebase:

static const struct bus_type ufs_rpmb_bus_type = {
	.name = "ufs_rpmb",
};

When ufs_rpmb_probe() assigns this unregistered bus type to ufs_rpmb->dev.bus:

	ufs_rpmb->dev.bus = &ufs_rpmb_bus_type;

does this guarantee that device_register() will deterministically fail when it attempts to retrieve the bus subsystem?

[ ... ]

drivers/ufs/core/ufs-rpmb.c static void ufs_rpmb_device_release(struct device *dev) · line 165

Quoted 8 lines
drivers/ufs/core/ufs-rpmb.c +1 -0
@@ -157,6 +159,7 @@ static void ufs_rpmb_device_release(struct device *dev)
 int ufs_rpmb_probe(struct ufs_hba *hba)
 {
 	struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
+	u8 dev_id[UFS_RPMB_ID_LEN];
 	struct rpmb_dev *rdev;
 	char *cid = NULL;
 	int region;

[Severity: High]
This isn't a bug introduced by this patch, but ufs_rpmb_device_release() just above here accesses memory that could have already been freed:

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);
}

Since ufs_rpmb_probe() allocates ufs_rpmb with devm_kzalloc() and subsequently calls device_register(&ufs_rpmb->dev), if the host driver unbinds, devres will free ufs_rpmb unconditionally. If a local user holds an open file descriptor or sysfs reference to the device, the kobject remains alive.

When the final reference is dropped, ufs_rpmb_device_release() is invoked, which then dereferences the already-freed ufs_rpmb memory. Can this cause a use-after-free?

[Severity: High]
This is also a pre-existing issue, but child unregistration is incorrectly placed inside the parent's release function:

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);
}

During driver removal, ufs_rpmb_remove() calls device_unregister(&ufs_rpmb->dev) on the parent before unregistering the child. Because the child still pins the parent's refcount, the parent's refcount never reaches zero, preventing its release function from executing.

Does this create an unbreakable circular reference that prevents device cleanup and permanently leaks both device structures?

[ ... ]

drivers/ufs/core/ufs-rpmb.c int ufs_rpmb_probe(struct ufs_hba *hba) · line 227

Quoted 13 lines
drivers/ufs/core/ufs-rpmb.c +4 -2
@@ -215,8 +218,10 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 			goto err_out;
 		}
 
-		descr.dev_id = cid;
-		descr.dev_id_len = strlen(cid);
+		blake2b(NULL, 0, cid, strlen(cid), dev_id, UFS_RPMB_ID_LEN);
+
+		descr.dev_id = dev_id;
+		descr.dev_id_len = UFS_RPMB_ID_LEN;
 		descr.capacity = cap;
 
 		/* Register RPMB device */

[Severity: High]
This isn't a bug introduced by this patch, but there is an initialization race when registering the RPMB device just below this block:

		/* Register RPMB device */
		rdev = rpmb_dev_register(&ufs_rpmb->dev, &descr);
		...
		ufs_rpmb->rdev = rdev;
		ufs_rpmb->region_id = region;

rpmb_dev_register() exposes the device to the rpmb_class, which synchronously triggers class notifiers. If a notifier like OP-TEE schedules a workqueue to probe the RPMB device, the worker could issue SMC calls resulting in an RPC back to ufs_rpmb_route_frames(), which relies on ufs_rpmb->region_id.

Since ufs_rpmb->region_id is initialized after rpmb_dev_register() returns, could the asynchronous probe thread read an uninitialized region_id for non-zero regions?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
maintainer Applied
#4 lore raw
In reply to Jorge Ramirez-Ortiz

Jorge,

Quoted 3 lines

This series makes UFS RPMB work out of the box with an OP-TEE that implements the standard eMMC RPMB key-derivation flow, without requiring any fundamental changes on the OP-TEE side.

Applied to 7.4/scsi-staging, thanks!

Signature · 1 line
Martin K. Petersen
maintainer Applied
#5 lore raw
In reply to Jorge Ramirez-Ortiz
Quoted 12 lines · Jorge Ramirez-Ortiz

This series makes UFS RPMB work out of the box with an OP-TEE that implements the standard eMMC RPMB key-derivation flow, without requiring any fundamental changes on the OP-TEE side.

RPMB provides an authenticated, replay-protected storage area whose security relies on a secret authentication key. In our setup that key is never exposed to the kernel: OP-TEE derives it in the secure world from its hardware-unique key and a device identifier (dev_id) that the RPMB core hands down. OP-TEE's implementation targets eMMC, where dev_id is the 16-byte eMMC CID, and both the fixed length and the raw-CID layout are baked into its key derivation.

Quoted 1 line

[...]

Applied to 7.4/scsi-queue, thanks!

[1/2] ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN

      https://git.kernel.org/mkp/scsi/c/a8a34238e5e6
[2/2] ufs: rpmb: use a fixed-length RPMB dev_id
      https://git.kernel.org/mkp/scsi/c/657eff806d38
Signature · 1 line
Martin K. Petersen