Date: Wed, 29 May 2024 13:36:26 +0530
From: Sumit Garg <sumit.garg@...aro.org>
To: Mikko Rapeli <mikko.rapeli@...aro.org>
Cc: Jens Wiklander <jens.wiklander@...aro.org>, 
	Jerome Forissier <jerome.forissier@...aro.org>, linux-kernel@...r.kernel.org, 
	linux-mmc@...r.kernel.org, op-tee@...ts.trustedfirmware.org, 
	Shyam Saini <shyamsaini@...ux.microsoft.com>, Ulf Hansson <ulf.hansson@...aro.org>, 
	Linus Walleij <linus.walleij@...aro.org>, Ilias Apalodimas <ilias.apalodimas@...aro.org>, 
	Bart Van Assche <bvanassche@....org>, Randy Dunlap <rdunlap@...radead.org>, 
	Ard Biesheuvel <ardb@...nel.org>, Arnd Bergmann <arnd@...db.de>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Manuel Traut <manut@...ka.net>
Subject: Re: [PATCH v7 4/4] optee: probe RPMB device using RPMB subsystem

On Wed, 29 May 2024 at 12:39, Mikko Rapeli <mikko.rapeli@...aro.org> wrote:
>
> Hi,
>
> On Wed, May 29, 2024 at 10:56:04AM +0530, Sumit Garg wrote:
> > Hi Mikko,
> >
> > On Tue, 28 May 2024 at 15:00, Mikko Rapeli <mikko.rapeli@...aro.org> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, May 27, 2024 at 03:24:01PM +0200, Jens Wiklander wrote:
> > > > On Mon, May 27, 2024 at 3:00 PM Jerome Forissier
> > > > <jerome.forissier@...aro.org> wrote:
> > > > >
> > > > > On 5/27/24 14:13, Jens Wiklander wrote:
> > > > > > Adds support in the OP-TEE drivers (both SMC and FF-A ABIs) to probe and
> > > > > > use an RPMB device via the RPMB subsystem instead of passing the RPMB
> > > > > > frames via tee-supplicant in user space. A fallback mechanism is kept to
> > > > > > route RPMB frames via tee-supplicant if the RPMB subsystem isn't
> > > > > > available.
> > > > > >
> > > > > > The OP-TEE RPC ABI is extended to support iterating over all RPMB
> > > > > > devices until one is found with the expected RPMB key already
> > > > > > programmed.
> > > > > >
> > > > > > Signed-off-by: Jens Wiklander <jens.wiklander@...aro.org>
> > > > > > Tested-by: Manuel Traut <manut@...ka.net>
> > > > > > ---
> > > > > >  Documentation/ABI/testing/sysfs-class-tee |  15 ++
> > > > > >  MAINTAINERS                               |   1 +
> > > > > >  drivers/tee/optee/core.c                  |  96 +++++++++++-
> > > > > >  drivers/tee/optee/device.c                |   7 +
> > > > > >  drivers/tee/optee/ffa_abi.c               |  14 ++
> > > > > >  drivers/tee/optee/optee_ffa.h             |   2 +
> > > > > >  drivers/tee/optee/optee_private.h         |  26 +++-
> > > > > >  drivers/tee/optee/optee_rpc_cmd.h         |  35 +++++
> > > > > >  drivers/tee/optee/optee_smc.h             |   2 +
> > > > > >  drivers/tee/optee/rpc.c                   | 177 ++++++++++++++++++++++
> > > > > >  drivers/tee/optee/smc_abi.c               |  14 ++
> > > > > >  11 files changed, 387 insertions(+), 2 deletions(-)
> > > > > >  create mode 100644 Documentation/ABI/testing/sysfs-class-tee
> > > > > >
> > > > > > diff --git a/Documentation/ABI/testing/sysfs-class-tee b/Documentation/ABI/testing/sysfs-class-tee
> > > > > > new file mode 100644
> > > > > > index 000000000000..c9144d16003e
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/ABI/testing/sysfs-class-tee
> > > > > > @@ -0,0 +1,15 @@
> > > > > > +What:                /sys/class/tee/tee{,priv}X/rpmb_routing_model
> > > > >
> > > > > Wouldn't /sys/class/tee/teeX/rpmb_routing_model be good enough?
> > > >
> > > > Doesn't the routing model concern tee-supplicant more than a TEE
> > > > client? Then it might make more sense to have
> > > > /sys/class/tee/teeprivX/rpmb_routing_model only. Keeping it for both
> > > > devices representing the same internal struct optee makes it easier to
> > > > find. Anyway, I don't mind removing one. Mikko, what do you prefer?
> > >
> > > As simple as possible. A single sysfs file is enough. Even the existence of the sysfs file
> > > could be the needed indicator for userspace to queue tee-supplicant startup.
> > >
> > > Outside of these patches, I think the optee RPC setup with fTPM TA is one area which
> > > currently requires tee-supplicant to be started. Detecting the existence of TPM before
> > > kernel drivers are loaded is possible via the exported EFI logs from firmware to kernel
> > > or ACPI TPM2 table entry, and detecting optee and thus starting tee-supplicant in userspace too.
> >
> > One thing I am trying to find an answer about is why do we need to
> > defer tee-supplicant launch if it's bundled into initrd? Once you
> > detect OP-TEE then tee-supplicant should be launched unconditionally.
> > As per your example below, the motivation here seems to be the TPM2
> > device dependent on RPMB backend but what if other future systemd
> > services come up and depend on other services offered by
> > tee-supplicant?
>
> There is an annoying depedency between firmware side optee and TAs, and kernel optee driver,
> tee-supplicant in userspace and kernel TA drivers like fTPM.
>
> Kernel fTPM driver and fTPM TA require tee-supplicant in userspace for RPMB, RPC etc.
>
> This patch series is adding kernel side support for RPMB handling so that the dependency to
> tee-supplicant in userspace can be removed. For fTPM use case, there is still the optee RPC
> buffer setup which currently requires tee-supplicant in userspace or fTPM TA will panic.
>
> So yes, currently, tee-supplicant must be started. But it would be great if kernel drivers
> and firmware optee trusted applications would not depend on tee-supplicant running in userspace.

Agree, we are aligned here. With this patch-set we aim to achieve
that, the user-space dependency for kernel drivers is hard to manage
and it's better to get rid of it. However, backwards compatibility for
distros will still require starting tee-supplicant.

> The startup sequence is really tricky to get right. My fTPM use case is using the TPM device
> to encrypt rootfs and thus all SW components including tee-supplicant need to run early in
> initramfs. Currently also switch from initramfs to main rootfs requires unloading
> fTPM kernel driver and stopping tee-supplicant in initrd, and then starting tee-supplicant
> and loading fTPM kernel driver from main rootfs. udev and automatic module loading for
> fTPM can not be used due to the tee-supplicant userspace dependency.

This is one of the reasons for gating tee-supplicant loading which I
was looking for. What happens if we want to keep tee-supplicant alive
when we migrate from initramfs to real rootfs? Does it work?

>
> As an example, here is v6 of this series on rockpi4b using fTPM TA with systemd based initrd
> without tee-supplicant and fTPM TA panics because the RPC setup is missing:

I think this is due to RPC allocation requested from tee-supplicant
during RPMB operations. Can you try following untested optee-os diff
and see if it resolves the problem for you?

diff --git a/core/tee/tee_rpmb_fs.c b/core/tee/tee_rpmb_fs.c
index 0ed30933b..b3d4d077a 100644
--- a/core/tee/tee_rpmb_fs.c
+++ b/core/tee/tee_rpmb_fs.c
@@ -418,11 +418,11 @@ static void tee_rpmb_free(struct tee_rpmb_mem *mem)
                return;

        if (mem->phreq_mobj) {
-               thread_rpc_free_payload(mem->phreq_mobj);
+               thread_rpc_free_kernel_payload(mem->phreq_mobj);
                mem->phreq_mobj = NULL;
        }
        if (mem->phresp_mobj) {
-               thread_rpc_free_payload(mem->phresp_mobj);
+               thread_rpc_free_kernel_payload(mem->phresp_mobj);
                mem->phresp_mobj = NULL;
        }
 }
@@ -440,8 +440,8 @@ static TEE_Result tee_rpmb_alloc(size_t req_size,
size_t resp_size,

        memset(mem, 0, sizeof(*mem));

-       mem->phreq_mobj = thread_rpc_alloc_payload(req_s);
-       mem->phresp_mobj = thread_rpc_alloc_payload(resp_s);
+       mem->phreq_mobj = thread_rpc_alloc_kernel_payload(req_s);
+       mem->phresp_mobj = thread_rpc_alloc_kernel_payload(resp_s);

        if (!mem->phreq_mobj || !mem->phresp_mobj) {
                res = TEE_ERROR_OUT_OF_MEMORY;

>
> https://ledge.validation.linaro.org/scheduler/job/87488
>
> [[0;32m  OK  [0m] Finished [0;1;39mFile System Check on /dev/mapper/usr[0m.
> E/TC:? 0 get_rpc_alloc_res:645 RPC allocation failed. Non-secure world result: ret=0xffff000c ret_origin=0x2
> E/TC:? 0
> E/TC:? 0 TA panicked with code 0xffff000c
> E/LD:  Status of TA bc50d971-d4c9-42c4-82cb-343fb7f37896
> E/LD:   arch: aarch64
> E/LD:  region  0: va 0x40005000 pa 0x3061b000 size 0x002000 flags rw-s (ldelf)
> E/LD:  region  1: va 0x40007000 pa 0x3061d000 size 0x008000 flags r-xs (ldelf)
> E/LD:  region  2: va 0x4000f000 pa 0x30625000 size 0x001000 flags rw-s (ldelf)
> E/LD:  region  3: va 0x40010000 pa 0x30626000 size 0x004000 flags rw-s (ldelf)
> E/LD:  region  4: va 0x40014000 pa 0x3062a000 size 0x001000 flags r--s
> E/LD:  region  5: va 0x40015000 pa 0x306b2000 size 0x011000 flags rw-s (stack)
> E/LD:  region  6: va 0x40026000 pa 0xe50ce000 size 0x002000 flags rw-- (param)
> E/LD:  region  7: va 0x40062000 pa 0x00001000 size 0x068000 flags r-xs [0]
> E/LD:  region  8: va 0x400ca000 pa 0x00069000 size 0x01f000 flags rw-s [0]
> E/LD:   [0] bc50d971-d4c9-42c4-82cb-343fb7f37896 @ 0x40062000
> E/LD:  Call stack:
> E/LD:   0x400a00c0
> E/LD:   0x40062b40
> E/LD:   0x400631b8
> E/LD:   0x40081f44
> E/LD:   0x4009b060
> E/LD:   0x40063a2c
> E/LD:   0x400a6298
> E/LD:   0x4009b214
> [    7.212584] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
> [    7.213281] tpm tpm0: tpm_try_transmit: send(): error -53212
> [    7.213820] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
> [    7.214493] tpm tpm0: tpm_try_transmit: send(): error -53212
> [    7.214996] optee-ftpm optee-ta-bc50d971-d4c9-42c4-82cb-343fb7f37896: ftpm_tee_probe: tpm_chip_register failed with rc=-53212
>          Mounting [0;1;39m/sysusr/usr[0m...
>
> This series adds the RPMB support in kernel, if HW supports it, but some HW doesn't and the
> tee-supplicant is emulating it as fall back.

That's just for testing purposes, right? It won't be used to implement
disk encryption for that HW.

> Userspace needs to know if tee-supplicant start
> is needed. Thus to me, exporting the RPMB routing details is useful for userspace.
>
> It's one thing to have a full control of HW and SW and force a policy, like always
> waiting for a specific TPM device to appear, but then again distros should be able
> to have automatic detection of TPM devices if firmware used them too and then
> start the needed bits in userspace, which depend on the firmware and HW configuration,
> like which SW components are needed for RPMB storage, kernel or tee-supplicant in userspace.
>
> These could possibly be just bugs in fTPM kernel driver and fTPM TA in optee world,
> which should be able to handle missing RPC and RPMB too and retry later on. Right now
> without tee-supplicant they panic early in boot and become unusable for the rest of the
> power cycle.

I very much would like to see fTPM working without dependency on
tee-supplicant. If there are any further problems you see then please
feel free to report.

-Sumit

>
> Cheers,
>
> -Mikko

Powered by blists - more mailing lists