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

On 20/07/26 11:16:11, Jorge Ramirez-Ortiz wrote:
> 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 blake2b 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. blake2b is used because it is
> already available in bootloaders such as U-Boot that must derive the
> same dev_id, avoiding the need to add a blake2s implementation there.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> ---
>  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..b62c00e7ff06 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_BLAKE2B 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 d0c7ea7a36f4..0d8399b6497a 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -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_SEC_PROTOCOL		0xEC	/* JEDEC UFS application */
>  #define UFS_RPMB_SEC_PROTOCOL_ID	0x01	/* JEDEC UFS RPMB protocol ID, CDB byte3 */
>  
> @@ -154,6 +156,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;
> @@ -213,8 +216,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;
> +		}
> +
> +		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 */
> @@ -228,6 +240,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;
> @@ -240,6 +254,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
> 

hi again Bean, any changes on this change? if so, would you mind updating it please?