linux-kernel - Re: [PATCH v2 3/3] scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHUa44HdV8FJMayVg6TFz7oGZc1b6QntxMsUN8mdTV7pm7vkKQ@mail.gmail.com>
Date: Mon, 13 Oct 2025 10:21:47 +0200
From: Jens Wiklander <jens.wiklander@...aro.org>
To: Bean Huo <huobean@...il.com>
Cc: avri.altman@....com, bvanassche@....org, alim.akhtar@...sung.com, 
	jejb@...ux.ibm.com, martin.petersen@...cle.com, can.guo@....qualcomm.com, 
	ulf.hansson@...aro.org, beanhuo@...ron.com, linux-scsi@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] scsi: ufs: core: Add OP-TEE based RPMB driver for
 UFS devices

Hi Bean,



On Wed, Oct 8, 2025 at 5:07 PM Bean Huo <huobean@...il.com> wrote:
>
> Jens,
>
> I incorporated your suggestions in my v3 excpet these two:
>
>
> On Wed, 2025-10-01 at 09:50 +0200, Jens Wiklander wrote:
> > > diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
> > > index cf820fa09a04..51e1867e524e 100644
> > > --- a/drivers/ufs/core/Makefile
> > > +++ b/drivers/ufs/core/Makefile
> > > @@ -2,6 +2,7 @@
> > >
> > >   obj-$(CONFIG_SCSI_UFSHCD)              += ufshcd-core.o
> > >   ufshcd-core-y                          += ufshcd.o ufs-sysfs.o ufs-mcq.o
> > > +ufshcd-core-$(CONFIG_RPMB)             += ufs-rpmb.o
> >
> > SCSI_UFSHCD might need the same trick ("depends on RPMB || !RPMB") in
> > Kconfig as we have for MMC_BLOCK.
> >
> > >
> When RPMB=m and SCSI_UFSHCD=y, the ufs-rpmb.o is compiled into the built-in
> ufshcd-core, ufs-rpmb.c calls functions from the OP-TEE RPMB subsystem module,
> The kernel allows built-in code to reference module symbols (they become runtime
> dependencies, not link-time), please check, I tested.
>
> > >
> > >
> >
> > > +
> > > +       struct rpmb_descr descr = {
> > > +               .type = RPMB_TYPE_UFS,
> >
> > We'll need another type if the device uses the extended RPMB frame
> > format. How about you clarify this, where RPMB_TYPE_UFS is defined to
> > avoid confusion?
>
> As ufs-bsg.c, we could use ARPMB_TYPE_UFS for UFS advanced RPMB frame, if it is
> RPMB, we take it as normal RPMB, the frame should be the same as MMC RPMB.

Isn't it a bit confusing to set the type to RPMB_TYPE_EMMC when it's
actually a UFS RPMB, even if it's supposedly compatible enough?

While the frame format works, I'm concerned about the CID. It's
essentially a namespace of its own for eMMC, and at least the OP-TEE
implementation makes assumptions about the format by masking out the
PRV (Product Revision) and CRC (CRC7 checksum) fields from the CID
when deriving the RPMB key. For this to work reliably, the CID must be
guaranteed to be unique per RPMB device.

>From what I understand, for UFS, the serial number is only guaranteed
to be unique if the manufacturer and the product name are taken into
account. Combined, these fields can be much larger than 16 bytes, and
we also have the partition number to consider.

By using RPMB_TYPE_UFS we can define a device ID tailored for UFS with
all the fields we need. Thoughts?

Cheers,
Jens