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

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

[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.

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
     blake2s, 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).

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 | 41 ++++++++++++++++++++++++++++++++++---
 2 files changed, 39 insertions(+), 3 deletions(-)

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

[Jorge Ramirez-Ortiz]

After a power cycle, the first command sent to a UFS logical unit
completes with CHECK CONDITION and a power-on UNIT ATTENTION (ASC
0x29). The SCSI core reports this to the caller instead of retrying
it. For the RPMB well-known LU, that first command is the first RPMB
frame sent after boot, so the frame fails. RPMB has no earlier,
guaranteed access that could clear the condition beforehand, so this
breaks RPMB on every cold boot.

Ask the SCSI core to retry the power-on UNIT ATTENTION on the RPMB WLUN
so that RPMB works from the very first access after a power cycle.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/core/ufs-rpmb.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index ffad049872b9..adbec80cf381 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -40,6 +40,25 @@ 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_sense_hdr sshdr;
+	/* Retry the power-on UNIT ATTENTION (ASC 0x29); the SCSI core does not. */
+	struct scsi_failure failure_defs[] = {
+		{
+			.sense = UNIT_ATTENTION,
+			.asc = SCMD_FAILURE_ASC_ANY,
+			.ascq = SCMD_FAILURE_ASCQ_ANY,
+			.allowed = 3,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		{}
+	};
+	struct scsi_failures failures = {
+		.failure_definitions = failure_defs,
+	};
+	const struct scsi_exec_args exec_args = {
+		.sshdr = &sshdr,
+		.failures = &failures,
+	};
 	u8 cdb[12] = { };
 
 	cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
@@ -48,7 +67,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 */
-- 
2.54.0

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

[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, and it derives the key on that
assumption.

The UFS RPMB id built here 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-CID eMMC ABI, requiring OP-TEE to be taught about variable-length
UFS ids.

Hash the UFS id into a fixed 16-byte dev_id with blake2s instead. This
keeps the derived key stable and unique per region while matching the
eMMC CID layout OP-TEE relies on, so the key-derivation ABI stays
identical and no OP-TEE change is needed.

Select CRYPTO_LIB_BLAKE2S when RPMB is enabled since ufs-rpmb.c now
depends on it.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/Kconfig         |  1 +
 drivers/ufs/core/ufs-rpmb.c | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index f662e7ce71f1..0a050565872d 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -7,6 +7,7 @@ menuconfig SCSI_UFSHCD
 	tristate "Universal Flash Storage Controller"
 	depends on SCSI && SCSI_DMA
 	depends on RPMB || !RPMB
+	select CRYPTO_LIB_BLAKE2S if RPMB
 	select PM_DEVFREQ
 	select DEVFREQ_GOV_SIMPLE_ONDEMAND
 	select NLS
diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
index adbec80cf381..a7595be0884c 100644
--- a/drivers/ufs/core/ufs-rpmb.c
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -10,6 +10,7 @@
  *	Can Guo <can.guo@oss.qualcomm.com>
  */
 
+#include <crypto/blake2s.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_SEC_PROTOCOL		0xEC	/* JEDEC UFS application */
 #define UFS_RPMB_SEC_PROTOCOL_ID	0x01	/* JEDEC UFS RPMB protocol ID, CDB byte3 */
 
@@ -156,6 +158,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 {
 	struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
 	struct rpmb_dev *rdev;
+	char *dev_id = NULL;
 	char *cid = NULL;
 	int region;
 	u32 cap;
@@ -215,8 +218,17 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 			goto err_out;
 		}
 
-		descr.dev_id = cid;
-		descr.dev_id_len = strlen(cid);
+		dev_id = kzalloc(UFS_RPMB_ID_LEN, GFP_KERNEL);
+		if (!dev_id) {
+			device_unregister(&ufs_rpmb->dev);
+			ret = -ENOMEM;
+			goto err_out;
+		}
+
+		blake2s(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 */
@@ -230,6 +242,8 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 
 		kfree(cid);
 		cid = NULL;
+		kfree(dev_id);
+		dev_id = NULL;
 
 		ufs_rpmb->rdev = rdev;
 		ufs_rpmb->region_id = region;
@@ -242,6 +256,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
 	return 0;
 err_out:
 	kfree(cid);
+	kfree(dev_id);
 	list_for_each_entry_safe(it, tmp, &hba->rpmbs, node) {
 		list_del(&it->node);
 		device_unregister(&it->dev);
-- 
2.54.0

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

[sashiko-bot]

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

New issues:
- [Medium] Selecting a non-existent Kconfig symbol (`CRYPTO_LIB_BLAKE2S`).

Pre-existing issues:
- [High] Circular reference between the parent `ufs_rpmb->dev` and the child RPMB device `rdev` prevents proper unregistration and causes a memory leak.
- [High] struct ufs_rpmb_dev is allocated with devm_kzalloc but contains a reference-counted struct device, leading to a Use-After-Free.
- [High] TOCTOU race: ufs_rpmb->region_id is initialized after rpmb_dev_register() completes, allowing concurrent requests to use a zeroed region ID.
- [High] ufs_rpmb_bus_type is assigned to the device but never registered, causing device_register() to unconditionally fail.
--

--- Patch [2]: [PATCH 2/2] ufs: rpmb: use a fixed-length RPMB dev_id ---
commit 3594b8409e2ab8cc7cc016e7f74ba42ec3857db5
Author: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Subject: ufs: rpmb: use a fixed-length RPMB dev_id

This commit changes the UFS RPMB ID generation to use a fixed 16-byte
Blake2s hash instead of a variable-length string. This aligns the UFS RPMB
ID with the eMMC CID layout, ensuring compatibility with OP-TEE.

> diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
> index f662e7ce71f1b..0a050565872d2 100644
> --- a/drivers/ufs/Kconfig
> +++ b/drivers/ufs/Kconfig
> @@ -7,6 +7,7 @@ menuconfig SCSI_UFSHCD
>  	tristate "Universal Flash Storage Controller"
>  	depends on SCSI && SCSI_DMA
>  	depends on RPMB || !RPMB
> +	select CRYPTO_LIB_BLAKE2S if RPMB
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Does the CRYPTO_LIB_BLAKE2S config symbol exist?

As documented in lib/crypto/Kconfig, BLAKE2s support is always built-in,
so this select statement appears to reference a non-existent symbol which
could cause unmet direct dependency warnings.

[ ... ]
> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index adbec80cf3811..a7595be0884c6 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
[ ... ]
> @@ -156,6 +158,7 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
>  {
>  	struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;

[Severity: High]
This isn't a bug introduced by this patch, but does devm_kzalloc() safely
manage the lifecycle of the reference-counted ufs_rpmb_dev structure?

In ufs_rpmb_probe():
    ufs_rpmb = devm_kzalloc(hba->dev, sizeof(*ufs_rpmb), GFP_KERNEL);
    ...
    ret = device_register(&ufs_rpmb->dev);

Because devm_kzalloc() ties the memory to the host device's devres, the
devres subsystem will automatically free ufs_rpmb during driver unbind. If
the device reference count is still elevated at that time (e.g. via sysfs),
could the release callback ufs_rpmb_device_release() later dereference this
freed memory when the reference count finally drops to zero?


[Severity: High]
This isn't a bug introduced by this patch, but does the UFS RPMB bus
registration fail unconditionally?

In ufs_rpmb_probe(), the bus is assigned:
    ufs_rpmb->dev.bus = &ufs_rpmb_bus_type;

However, ufs_rpmb_bus_type appears to be statically defined and never
registered via bus_register(). When device_register(&ufs_rpmb->dev) is
called, will it fail with -EINVAL because the driver core cannot find the
bus subsystem?

[ ... ]
> @@ -230,6 +242,8 @@ int ufs_rpmb_probe(struct ufs_hba *hba)
>  
>  		kfree(cid);
>  		cid = NULL;
> +		kfree(dev_id);
> +		dev_id = NULL;
>  
>  		ufs_rpmb->rdev = rdev;

[Severity: High]
This is a pre-existing issue, but does this create a circular reference
that prevents the device from ever being unregistered?

ufs_rpmb_probe() registers a child device:
    rdev = rpmb_dev_register(&ufs_rpmb->dev, &descr);

The child holds a reference to the parent. In ufs_rpmb_remove(), the parent
is unregistered without explicitly unregistering the child:
    device_unregister(&ufs_rpmb->dev);

Since the release callback ufs_rpmb_device_release() contains the call to
rpmb_dev_unregister(ufs_rpmb->rdev), it will never be called because the
child keeps the parent's reference count elevated. Can this lead to a
permanent memory leak of the device?

>  		ufs_rpmb->region_id = region;

[Severity: High]
This is a pre-existing issue, but is there a race condition between
registering the RPMB device and setting the region_id?

In ufs_rpmb_probe():
    rdev = rpmb_dev_register(&ufs_rpmb->dev, &descr);
    ...
    ufs_rpmb->region_id = region;

When rpmb_dev_register() completes, the RPMB class interface's add_dev
callback could schedule a background worker to access the device. If this
worker issues requests before region_id is initialized, could
ufs_rpmb_route_frames() read a zero-initialized region_id and incorrectly
route requests?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715214327.1933560-1-jorge.ramirez@oss.qualcomm.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The code uses SCMD_FAILURE_ASC_ANY to retry any Unit Attention, contradicting the documented intent.
- [Low] The 'sshdr' variable is allocated and passed to scsi_execute_cmd but is never used by the caller.
--

--- Patch [1]: [PATCH 1/2] ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN ---
commit 7f3e584cbe66720f776d9dd47015f86621c3fdb8
Author: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>

ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN

After a power cycle, the first command to the RPMB WLUN fails with a
power-on UNIT ATTENTION. This patch asks the SCSI core to retry this
condition so RPMB works immediately after boot.

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index ffad049872b9a..adbec80cf3811 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -40,6 +40,25 @@ 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_sense_hdr sshdr;

[Severity: Low]
Does this code need to allocate and pass sshdr?

It appears that sshdr is passed to scsi_execute_cmd() via exec_args but the
result is never read by the caller. Leaving .sshdr as NULL would avoid the
unnecessary sense data normalization overhead in scsi_execute_cmd().

> +	/* Retry the power-on UNIT ATTENTION (ASC 0x29); the SCSI core does not. */
> +	struct scsi_failure failure_defs[] = {
> +		{
> +			.sense = UNIT_ATTENTION,
> +			.asc = SCMD_FAILURE_ASC_ANY,

[Severity: Medium]
Will this retry ANY Unit Attention condition instead of just ASC 0x29?

The commit message and inline comment explicitly target the power-on
UNIT ATTENTION (ASC 0x29), but using SCMD_FAILURE_ASC_ANY configures the SCSI
core to retry any Unit Attention condition up to 3 times. Could this mask
other unexpected device state changes?

> +			.ascq = SCMD_FAILURE_ASCQ_ANY,
> +			.allowed = 3,
> +			.result = SAM_STAT_CHECK_CONDITION,
> +		},
> +		{}
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715214327.1933560-1-jorge.ramirez@oss.qualcomm.com?part=1