2024-04-22 11:27:54

by Mikko Rapeli

[permalink] [raw]
Subject: [PATCH] efi: expose TPM event log to userspace via sysfs

Userspace needs to know if TPM kernel drivers need to be loaded
and related services started early in the boot if TPM device
is used and available. If EFI firmware has used TPM device
to e.g. measure binaries, then many of them also provide the TPM
log to kernel in addition to the actual TPM device side measurements.
Expose availability of TPM event log to userspace via
/sys/firmware/efi/tpm_log. If the file exists, then firmware
provided a TPM event log to kernel, and userspace init should also
queue TPM module loading and other early boot services for TPM support.

Enables systemd to support TPM drivers as modules when rootfs is
encrypted with the TPM device.

Sample output from a arm64 qemu machine with u-boot based EFI firmware
and swtpm:

root@trs-qemuarm64:~# dmesg|grep TPMEvent
[ 0.000000] efi: TPMFinalLog=0xbd648040 RTPROP=0xbd646040 SMBIOS3.0=0xbe6ad000 TPMEventLog=0xbd5f9040 INITRD=0xbd5f7040 RNG=0xbd5f6040 MEMRESERVE=0xbd5f5040
root@trs-qemuarm64:~# ls -l /sys/firmware/efi/tpm_log
-r-------- 1 root root 4096 Apr 22 10:31 /sys/firmware/efi/tpm_log
root@trs-qemuarm64:~# cat /sys/firmware/efi/tpm_log
TPMEventLog=0xbd5f9040
root@trs-qemuarm64:~# cat /sys/firmware/efi/systab
SMBIOS3=0xbe6ad000

Other similar information is currently in /sys/firmware/efi/systab but
for new exported variables a one-variable-per-file sysfs interface
is preferred according to comments in systab_show()
drivers/firmware/efi/efi.c

See also:
https://github.com/systemd/systemd/pull/32314
https://lists.freedesktop.org/archives/systemd-devel/2024-April/050206.html

Cc: Ilias Apalodimas <[email protected]>
Cc: Lennart Poettering <[email protected]>
Signed-off-by: Mikko Rapeli <[email protected]>
---
Documentation/ABI/testing/sysfs-firmware-efi | 12 ++++++++++++
drivers/firmware/efi/efi.c | 13 +++++++++++++
2 files changed, 25 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi
index 5e4d0b27cdfe..caaff27cc73e 100644
--- a/Documentation/ABI/testing/sysfs-firmware-efi
+++ b/Documentation/ABI/testing/sysfs-firmware-efi
@@ -36,3 +36,15 @@ Description: Displays the content of the Runtime Configuration Interface
Table version 2 on Dell EMC PowerEdge systems in binary format
Users: It is used by Dell EMC OpenManage Server Administrator tool to
populate BIOS setup page.
+
+What: /sys/firmware/efi/tpm_log
+Date: April 2024
+Contact: Mikko Rapeli <[email protected]>
+Description: If EFI firmware supports TPM device and measurements were done
+ then a TPM event log has very likely been generated and provided
+ to the kernel. This serves as indicator for userspace to load
+ TPM drivers and to start related service early in the boot sequence,
+ e.g. initramfs, where full bus probes and device scans are not yet
+ done.
+Users: systemd will use this interface to support TPM drivers as modules also
+ for early initramfs
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 4fcda50acfa4..94773e8b8806 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -162,6 +162,13 @@ static ssize_t fw_platform_size_show(struct kobject *kobj,
return sprintf(buf, "%d\n", efi_enabled(EFI_64BIT) ? 64 : 32);
}

+static ssize_t tpm_log_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ return sprintf(buf, "TPMEventLog=0x%lx", efi.tpm_log);
+}
+static struct kobj_attribute efi_attr_tpm_log = __ATTR_RO_MODE(tpm_log, 0400);
+
extern __weak struct kobj_attribute efi_attr_fw_vendor;
extern __weak struct kobj_attribute efi_attr_runtime;
extern __weak struct kobj_attribute efi_attr_config_table;
@@ -459,6 +466,12 @@ static int __init efisubsys_init(void)
platform_device_register_simple("efi_secret", 0, NULL, 0);
#endif

+ if (efi.tpm_log != EFI_INVALID_TABLE_ADDR) {
+ error = sysfs_create_file(efi_kobj, &efi_attr_tpm_log.attr);
+ if (error)
+ pr_err("sysfs create file failed with error %d.\n", error);
+ }
+
return 0;

err_remove_group:
--
2.34.1



2024-04-22 12:48:27

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> Userspace needs to know if TPM kernel drivers need to be loaded
> and related services started early in the boot if TPM device
> is used and available.

This says what but not why. We already have module autoloading that
works correctly for TPM devices, so why is this needed?

We do have a chicken and egg problem with IMA in that the TPM driver
needs to be present *before* any filesystem, including the one the TPM
modules would be on, is mounted so executions can be measured into IMA
(meaning that if you use IMA the TPM drivers must be built in) but this
sounds to be something different. However, because of the IMA problem,
most distributions don't end up compiling TPM drivers as modules
anyway.

Is what you want simply that tpm modules be loaded earlier?

James


2024-04-22 13:09:05

by Mikko Rapeli

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi,

On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley wrote:
> On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > Userspace needs to know if TPM kernel drivers need to be loaded
> > and related services started early in the boot if TPM device
> > is used and available.
>
> This says what but not why. We already have module autoloading that
> works correctly for TPM devices, so why is this needed?
>
> We do have a chicken and egg problem with IMA in that the TPM driver
> needs to be present *before* any filesystem, including the one the TPM
> modules would be on, is mounted so executions can be measured into IMA
> (meaning that if you use IMA the TPM drivers must be built in) but this
> sounds to be something different. However, because of the IMA problem,
> most distributions don't end up compiling TPM drivers as modules
> anyway.
>
> Is what you want simply that tpm modules be loaded earlier?

Yes, ealier is the problem. In my specific testing case the machine is
qemu arm64 with swtpm with EFI firmware for secure boot and TPM support.

Firmware uses TPM and does measurements and thus TPM event log is available
on this machine and a bunch of other arm64 boards. Visible in early boot
dmesg as TPMEventLog lines like:

[ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040 RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040 INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040

Different boards use different TPM HW and drivers so compiling all these in is
possible but a bit ugly. systemd recently gained support for a specific
tpm2.target which makes TPM support modular and also works with kernel modules for some
TPM use cases but not rootfs encryption.

In my test case we have a kernel+initramfs uki binary which is loaded by EFI firmware
as a secure boot binary. TPM support on various boards is visible in devicetree but
not as ACPI table entries. systemd currently detect TPM2 support either via ACPI table
/sys/firmware/acpi/tables/TPM2 or TPM entry or via firmware measurement via
/sys/kernel/security/tpm0/binary_bios_measurements . If either one of these exist,
then systemd evaluates ConditionSecurity=measured-uki in services correctly and
rolls out TPM services, cryptsetup etc. But the ACPI table entry for TPM isn't mandatory
and many boards don't support it. Then latter requies TPM kernel driver to be loaded
before systemd evaluates ConditionSecurity=measured-uki the first time, or basically
the driver needs to be compiled into the kernel.

In my case the uki initramfs is also based on systemd and does things like
creating a TPM encrypted rootfs and this should work on a number of boards
automatically, and none of the boards have ACPI table entries for TPM2, but
all of them with real, swtpm or fTPM based TPM devices provide the TPM Event Log
to the kernel. Thus systemd could use this as an indicator for TPM support in addition
to the server grade HW standard ACPI table entry. And once this is in place
TPM drivers and module loading work and initramfs can create a TPM backed rootfs
on first boot. The catch is to install needed kernel modules to the initramfs but
after that, all things work nicely.

Hope this clarifies this change a bit more.

Cheers,

-Mikko

2024-04-22 13:34:21

by Ilias Apalodimas

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi all,

On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli <[email protected]> wrote:
>
> Hi,
>
> On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley wrote:
> > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > Userspace needs to know if TPM kernel drivers need to be loaded
> > > and related services started early in the boot if TPM device
> > > is used and available.
> >
> > This says what but not why. We already have module autoloading that
> > works correctly for TPM devices, so why is this needed?
> >
> > We do have a chicken and egg problem with IMA in that the TPM driver
> > needs to be present *before* any filesystem, including the one the TPM
> > modules would be on, is mounted so executions can be measured into IMA
> > (meaning that if you use IMA the TPM drivers must be built in) but this
> > sounds to be something different. However, because of the IMA problem,
> > most distributions don't end up compiling TPM drivers as modules
> > anyway.
> >
> > Is what you want simply that tpm modules be loaded earlier?
>
> Yes, ealier is the problem. In my specific testing case the machine is
> qemu arm64 with swtpm with EFI firmware for secure boot and TPM support.
>
> Firmware uses TPM and does measurements and thus TPM event log is available
> on this machine and a bunch of other arm64 boards. Visible in early boot
> dmesg as TPMEventLog lines like:
>
> [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040 RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040 INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
>
> Different boards use different TPM HW and drivers so compiling all these in is
> possible but a bit ugly. systemd recently gained support for a specific
> tpm2.target which makes TPM support modular and also works with kernel modules for some
> TPM use cases but not rootfs encryption.
>
> In my test case we have a kernel+initramfs uki binary which is loaded by EFI firmware
> as a secure boot binary. TPM support on various boards is visible in devicetree but
> not as ACPI table entries. systemd currently detect TPM2 support either via ACPI table
> /sys/firmware/acpi/tables/TPM2 or TPM entry or via firmware measurement via
> /sys/kernel/security/tpm0/binary_bios_measurements .

One corner case worth noting here is that scanning the device tree
won't always work for non-ACPI systems... The reason is that a
firmware TPM (running in OP-TEE) might or might not have a DT entry,
since OP-TEE can discover the device dynamically and doesn't always
rely on a DT entry.

I don't particularly love the idea that an EventLog existence
automatically means a TPM will be there, but it seems that systemd
already relies on that and it does solve the problem we have.

/Ilias


> If either one of these exist,
> then systemd evaluates ConditionSecurity=measured-uki in services correctly and
> rolls out TPM services, cryptsetup etc. But the ACPI table entry for TPM isn't mandatory
> and many boards don't support it. Then latter requies TPM kernel driver to be loaded
> before systemd evaluates ConditionSecurity=measured-uki the first time, or basically
> the driver needs to be compiled into the kernel.
>
> In my case the uki initramfs is also based on systemd and does things like
> creating a TPM encrypted rootfs and this should work on a number of boards
> automatically, and none of the boards have ACPI table entries for TPM2, but
> all of them with real, swtpm or fTPM based TPM devices provide the TPM Event Log
> to the kernel. Thus systemd could use this as an indicator for TPM support in addition
> to the server grade HW standard ACPI table entry. And once this is in place
> TPM drivers and module loading work and initramfs can create a TPM backed rootfs
> on first boot. The catch is to install needed kernel modules to the initramfs but
> after that, all things work nicely.
>
> Hope this clarifies this change a bit more.
>
> Cheers,
>
> -Mikko

2024-04-22 13:39:35

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> Hi all,
>
> On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli <[email protected]>
> wrote:
> >
> > Hi,
> >
> > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley wrote:
> > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > Userspace needs to know if TPM kernel drivers need to be loaded
> > > > and related services started early in the boot if TPM device
> > > > is used and available.
> > >
> > > This says what but not why.  We already have module autoloading
> > > that works correctly for TPM devices, so why is this needed?
> > >
> > > We do have a chicken and egg problem with IMA in that the TPM
> > > driver needs to be present *before* any filesystem, including the
> > > one the TPM modules would be on, is mounted so executions can be
> > > measured into IMA (meaning that if you use IMA the TPM drivers
> > > must be built in) but this sounds to be something different.
> > > However, because of the IMA problem, most distributions don't end
> > > up compiling TPM drivers as modules anyway.
> > >
> > > Is what you want simply that tpm modules be loaded earlier?
> >
> > Yes, ealier is the problem. In my specific testing case the machine
> > is qemu arm64 with swtpm with EFI firmware for secure boot and TPM
> > support.
> >
> > Firmware uses TPM and does measurements and thus TPM event log is
> > available on this machine and a bunch of other arm64 boards.
> > Visible in early boot dmesg as TPMEventLog lines like:
> >
> > [    0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> >
> > Different boards use different TPM HW and drivers so compiling all
> > these in is possible but a bit ugly. systemd recently gained
> > support for a specific tpm2.target which makes TPM support modular
> > and also works with kernel modules for some TPM use cases but not
> > rootfs encryption.
> >
> > In my test case we have a kernel+initramfs uki binary which is
> > loaded by EFI firmware as a secure boot binary. TPM support on
> > various boards is visible in devicetree but not as ACPI table
> > entries. systemd currently detect TPM2 support either via ACPI
> > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via firmware
> > measurement via /sys/kernel/security/tpm0/binary_bios_measurements
> > .
>
> One corner case worth noting here is that scanning the device tree
> won't always work for non-ACPI systems... The reason is that a
> firmware TPM (running in OP-TEE) might or might not have a DT entry,
> since OP-TEE can discover the device dynamically and doesn't always
> rely on a DT entry.
>
> I don't particularly love the idea that an EventLog existence
> automatically means a TPM will be there, but it seems that systemd
> already relies on that and it does solve the problem we have.

Well, quite. That's why the question I was interested in, perhaps not
asked as clearly as it could be is: since all the TPM devices rely on
discovery mechanisms like ACPI or DT or the like which are ready quite
early, should we simply be auto loading the TPM drivers earlier?

James


2024-04-22 14:13:27

by Ilias Apalodimas

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi James

On Mon, 22 Apr 2024 at 16:38, James Bottomley
<[email protected]> wrote:
>
> On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> > Hi all,
> >
> > On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli <[email protected]>
> > wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley wrote:
> > > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > > Userspace needs to know if TPM kernel drivers need to be loaded
> > > > > and related services started early in the boot if TPM device
> > > > > is used and available.
> > > >
> > > > This says what but not why. We already have module autoloading
> > > > that works correctly for TPM devices, so why is this needed?
> > > >
> > > > We do have a chicken and egg problem with IMA in that the TPM
> > > > driver needs to be present *before* any filesystem, including the
> > > > one the TPM modules would be on, is mounted so executions can be
> > > > measured into IMA (meaning that if you use IMA the TPM drivers
> > > > must be built in) but this sounds to be something different.
> > > > However, because of the IMA problem, most distributions don't end
> > > > up compiling TPM drivers as modules anyway.
> > > >
> > > > Is what you want simply that tpm modules be loaded earlier?
> > >
> > > Yes, ealier is the problem. In my specific testing case the machine
> > > is qemu arm64 with swtpm with EFI firmware for secure boot and TPM
> > > support.
> > >
> > > Firmware uses TPM and does measurements and thus TPM event log is
> > > available on this machine and a bunch of other arm64 boards.
> > > Visible in early boot dmesg as TPMEventLog lines like:
> > >
> > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > >
> > > Different boards use different TPM HW and drivers so compiling all
> > > these in is possible but a bit ugly. systemd recently gained
> > > support for a specific tpm2.target which makes TPM support modular
> > > and also works with kernel modules for some TPM use cases but not
> > > rootfs encryption.
> > >
> > > In my test case we have a kernel+initramfs uki binary which is
> > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > various boards is visible in devicetree but not as ACPI table
> > > entries. systemd currently detect TPM2 support either via ACPI
> > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via firmware
> > > measurement via /sys/kernel/security/tpm0/binary_bios_measurements
> > > .
> >
> > One corner case worth noting here is that scanning the device tree
> > won't always work for non-ACPI systems... The reason is that a
> > firmware TPM (running in OP-TEE) might or might not have a DT entry,
> > since OP-TEE can discover the device dynamically and doesn't always
> > rely on a DT entry.
> >
> > I don't particularly love the idea that an EventLog existence
> > automatically means a TPM will be there, but it seems that systemd
> > already relies on that and it does solve the problem we have.
>
> Well, quite. That's why the question I was interested in, perhaps not
> asked as clearly as it could be is: since all the TPM devices rely on
> discovery mechanisms like ACPI or DT or the like which are ready quite
> early, should we simply be auto loading the TPM drivers earlier?

This would be an elegant way to solve this and on top of that, we have
a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
But to answer that we need more feedback from systemd. What 'earlier'
means? Autload it from the kernel before we go into launching the
initrd?

Thanks
/Ilias
> James
>

2024-04-22 14:31:44

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon, 2024-04-22 at 16:54 +0300, Ilias Apalodimas wrote:
> Hi James
>
> On Mon, 22 Apr 2024 at 16:38, James Bottomley
> <[email protected]> wrote:
> >
> > On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> > > Hi all,
> > >
> > > On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli
> > > <[email protected]>
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley
> > > > wrote:
> > > > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > > > Userspace needs to know if TPM kernel drivers need to be
> > > > > > loaded and related services started early in the boot if
> > > > > > TPM device is used and available.
> > > > >
> > > > > This says what but not why.  We already have module
> > > > > autoloading that works correctly for TPM devices, so why is
> > > > > this needed?
> > > > >
> > > > > We do have a chicken and egg problem with IMA in that the TPM
> > > > > driver needs to be present *before* any filesystem, including
> > > > > the one the TPM modules would be on, is mounted so executions
> > > > > can be measured into IMA (meaning that if you use IMA the TPM
> > > > > drivers must be built in) but this sounds to be something
> > > > > different. However, because of the IMA problem, most
> > > > > distributions don't end up compiling TPM drivers as modules
> > > > > anyway.
> > > > >
> > > > > Is what you want simply that tpm modules be loaded earlier?
> > > >
> > > > Yes, ealier is the problem. In my specific testing case the
> > > > machine is qemu arm64 with swtpm with EFI firmware for secure
> > > > boot and TPM support.
> > > >
> > > > Firmware uses TPM and does measurements and thus TPM event log
> > > > is
> > > > available on this machine and a bunch of other arm64 boards.
> > > > Visible in early boot dmesg as TPMEventLog lines like:
> > > >
> > > > [    0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > >
> > > > Different boards use different TPM HW and drivers so compiling
> > > > all these in is possible but a bit ugly. systemd recently
> > > > gained support for a specific tpm2.target which makes TPM
> > > > support modular and also works with kernel modules for some TPM
> > > > use cases but not rootfs encryption.
> > > >
> > > > In my test case we have a kernel+initramfs uki binary which is
> > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > various boards is visible in devicetree but not as ACPI table
> > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via
> > > > firmware measurement via
> > > > /sys/kernel/security/tpm0/binary_bios_measurements
> > > > .
> > >
> > > One corner case worth noting here is that scanning the device
> > > tree won't always work for non-ACPI systems... The reason is that
> > > a firmware TPM (running in OP-TEE) might or might not have a DT
> > > entry, since OP-TEE can discover the device dynamically and
> > > doesn't always rely on a DT entry.
> > >
> > > I don't particularly love the idea that an EventLog existence
> > > automatically means a TPM will be there, but it seems that
> > > systemd already relies on that and it does solve the problem we
> > > have.
> >
> > Well, quite. That's why the question I was interested in, perhaps
> > not asked as clearly as it could be is: since all the TPM devices
> > rely on discovery mechanisms like ACPI or DT or the like which are
> > ready quite early, should we simply be auto loading the TPM drivers
> > earlier?
>
> This would be an elegant way to solve this and on top of that, we
> have a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> But to answer that we need more feedback from systemd. What 'earlier'
> means? Autload it from the kernel before we go into launching the
> initrd?

Right, so this is another timing problem: we can't autoload modules
*before* they appear in the filesystem and presumably they're on the
initrd, so auto loading must be post initrd mount (and init execution)
but otherwise quite early?

This might be quite a bit of work. Logically, just moving the matching
and loading code earlier might work, but we used to have a
load_default_modules() at the end of init/main.c and it got removed
(because it only eventually loaded elevator modules) everything is now
loaded in it's various init routines, so to get, say, TPM ACPI modules
loaded earlier, we'd have to run the ACPI device matching code earlier
and so on for every other subsystem ...

James


2024-04-22 14:58:03

by Mikko Rapeli

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi,

On Mon, Apr 22, 2024 at 04:54:26PM +0300, Ilias Apalodimas wrote:
> Hi James
>
> On Mon, 22 Apr 2024 at 16:38, James Bottomley
> <[email protected]> wrote:
> >
> > On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> > > Hi all,
> > >
> > > On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli <[email protected]>
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley wrote:
> > > > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > > > Userspace needs to know if TPM kernel drivers need to be loaded
> > > > > > and related services started early in the boot if TPM device
> > > > > > is used and available.
> > > > >
> > > > > This says what but not why. We already have module autoloading
> > > > > that works correctly for TPM devices, so why is this needed?
> > > > >
> > > > > We do have a chicken and egg problem with IMA in that the TPM
> > > > > driver needs to be present *before* any filesystem, including the
> > > > > one the TPM modules would be on, is mounted so executions can be
> > > > > measured into IMA (meaning that if you use IMA the TPM drivers
> > > > > must be built in) but this sounds to be something different.
> > > > > However, because of the IMA problem, most distributions don't end
> > > > > up compiling TPM drivers as modules anyway.
> > > > >
> > > > > Is what you want simply that tpm modules be loaded earlier?
> > > >
> > > > Yes, ealier is the problem. In my specific testing case the machine
> > > > is qemu arm64 with swtpm with EFI firmware for secure boot and TPM
> > > > support.
> > > >
> > > > Firmware uses TPM and does measurements and thus TPM event log is
> > > > available on this machine and a bunch of other arm64 boards.
> > > > Visible in early boot dmesg as TPMEventLog lines like:
> > > >
> > > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > >
> > > > Different boards use different TPM HW and drivers so compiling all
> > > > these in is possible but a bit ugly. systemd recently gained
> > > > support for a specific tpm2.target which makes TPM support modular
> > > > and also works with kernel modules for some TPM use cases but not
> > > > rootfs encryption.
> > > >
> > > > In my test case we have a kernel+initramfs uki binary which is
> > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > various boards is visible in devicetree but not as ACPI table
> > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via firmware
> > > > measurement via /sys/kernel/security/tpm0/binary_bios_measurements
> > > > .
> > >
> > > One corner case worth noting here is that scanning the device tree
> > > won't always work for non-ACPI systems... The reason is that a
> > > firmware TPM (running in OP-TEE) might or might not have a DT entry,
> > > since OP-TEE can discover the device dynamically and doesn't always
> > > rely on a DT entry.
> > >
> > > I don't particularly love the idea that an EventLog existence
> > > automatically means a TPM will be there, but it seems that systemd
> > > already relies on that and it does solve the problem we have.
> >
> > Well, quite. That's why the question I was interested in, perhaps not
> > asked as clearly as it could be is: since all the TPM devices rely on
> > discovery mechanisms like ACPI or DT or the like which are ready quite
> > early, should we simply be auto loading the TPM drivers earlier?
>
> This would be an elegant way to solve this and on top of that, we have
> a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> But to answer that we need more feedback from systemd. What 'earlier'
> means? Autload it from the kernel before we go into launching the
> initrd?

This is sort of what already happens, but the question is when
does init/systemd wait for the TPM device discovery and setup
of related service so that rootfs can be mounted?

Currently the answer is, for device auto discover: when ACPI TPM2 table
exists or TPM kernel driver interface for firmware measurement is available.

Or as policy, when the kernel command line includes something or services
in initramfs are hard coded.

Parsing devicetree is really hard in userspace but it may contain the TPM details.
But the TPM can also be on some other discoverable bus, firmware TPM optee trusted
application. These both get discovered via the TPM Event Log if firmware
has TPM support on the arm64 boards we have.

Cheers,

-Mikko

2024-04-22 15:23:33

by Ilias Apalodimas

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon, 22 Apr 2024 at 17:31, James Bottomley
<[email protected]> wrote:
>
> On Mon, 2024-04-22 at 16:54 +0300, Ilias Apalodimas wrote:
> > Hi James
> >
> > On Mon, 22 Apr 2024 at 16:38, James Bottomley
> > <[email protected]> wrote:
> > >
> > > On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> > > > Hi all,
> > > >
> > > > On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli
> > > > <[email protected]>
> > > > wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley
> > > > > wrote:
> > > > > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > > > > Userspace needs to know if TPM kernel drivers need to be
> > > > > > > loaded and related services started early in the boot if
> > > > > > > TPM device is used and available.
> > > > > >
> > > > > > This says what but not why. We already have module
> > > > > > autoloading that works correctly for TPM devices, so why is
> > > > > > this needed?
> > > > > >
> > > > > > We do have a chicken and egg problem with IMA in that the TPM
> > > > > > driver needs to be present *before* any filesystem, including
> > > > > > the one the TPM modules would be on, is mounted so executions
> > > > > > can be measured into IMA (meaning that if you use IMA the TPM
> > > > > > drivers must be built in) but this sounds to be something
> > > > > > different. However, because of the IMA problem, most
> > > > > > distributions don't end up compiling TPM drivers as modules
> > > > > > anyway.
> > > > > >
> > > > > > Is what you want simply that tpm modules be loaded earlier?
> > > > >
> > > > > Yes, ealier is the problem. In my specific testing case the
> > > > > machine is qemu arm64 with swtpm with EFI firmware for secure
> > > > > boot and TPM support.
> > > > >
> > > > > Firmware uses TPM and does measurements and thus TPM event log
> > > > > is
> > > > > available on this machine and a bunch of other arm64 boards.
> > > > > Visible in early boot dmesg as TPMEventLog lines like:
> > > > >
> > > > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > > >
> > > > > Different boards use different TPM HW and drivers so compiling
> > > > > all these in is possible but a bit ugly. systemd recently
> > > > > gained support for a specific tpm2.target which makes TPM
> > > > > support modular and also works with kernel modules for some TPM
> > > > > use cases but not rootfs encryption.
> > > > >
> > > > > In my test case we have a kernel+initramfs uki binary which is
> > > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > > various boards is visible in devicetree but not as ACPI table
> > > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via
> > > > > firmware measurement via
> > > > > /sys/kernel/security/tpm0/binary_bios_measurements
> > > > > .
> > > >
> > > > One corner case worth noting here is that scanning the device
> > > > tree won't always work for non-ACPI systems... The reason is that
> > > > a firmware TPM (running in OP-TEE) might or might not have a DT
> > > > entry, since OP-TEE can discover the device dynamically and
> > > > doesn't always rely on a DT entry.
> > > >
> > > > I don't particularly love the idea that an EventLog existence
> > > > automatically means a TPM will be there, but it seems that
> > > > systemd already relies on that and it does solve the problem we
> > > > have.
> > >
> > > Well, quite. That's why the question I was interested in, perhaps
> > > not asked as clearly as it could be is: since all the TPM devices
> > > rely on discovery mechanisms like ACPI or DT or the like which are
> > > ready quite early, should we simply be auto loading the TPM drivers
> > > earlier?
> >
> > This would be an elegant way to solve this and on top of that, we
> > have a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> > But to answer that we need more feedback from systemd. What 'earlier'
> > means? Autload it from the kernel before we go into launching the
> > initrd?
>
> Right, so this is another timing problem: we can't autoload modules
> *before* they appear in the filesystem and presumably they're on the
> initrd, so auto loading must be post initrd mount (and init execution)
> but otherwise quite early?

Exactly. But is that enough?

>
> This might be quite a bit of work. Logically, just moving the matching
> and loading code earlier might work, but we used to have a
> load_default_modules() at the end of init/main.c and it got removed
> (because it only eventually loaded elevator modules) everything is now
> loaded in it's various init routines, so to get, say, TPM ACPI modules
> loaded earlier, we'd have to run the ACPI device matching code earlier
> and so on for every other subsystem ...

Being the devil's advocate here and as I stated I don't love this but ...
The kernel isn't technically doing anything wrong. We just expose an
*existing* EFI config table. The kernel also exposes filesystems so
people are free to do rm -rf *.
The fact that applications might use it as a means of "oh there's
probably a TPM" shouldn't be the end of the world. On top of that,
since it's an EFI config table we can keep it around and never break
any ABIs we create to userspace. If in the future we find a better
way, userspace can use that?

So perhaps this is ok as long as make sure we understand why systemd
needs it that early?

Thanks
/Ilias
>
> James
>

2024-04-24 17:15:59

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon, 22 Apr 2024 at 17:22, Ilias Apalodimas
<[email protected]> wrote:
>
> On Mon, 22 Apr 2024 at 17:31, James Bottomley
> <[email protected]> wrote:
> >
> > On Mon, 2024-04-22 at 16:54 +0300, Ilias Apalodimas wrote:
> > > Hi James
> > >
> > > On Mon, 22 Apr 2024 at 16:38, James Bottomley
> > > <[email protected]> wrote:
> > > >
> > > > On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> > > > > Hi all,
> > > > >
> > > > > On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli
> > > > > <[email protected]>
> > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley
> > > > > > wrote:
> > > > > > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > > > > > Userspace needs to know if TPM kernel drivers need to be
> > > > > > > > loaded and related services started early in the boot if
> > > > > > > > TPM device is used and available.
> > > > > > >
> > > > > > > This says what but not why. We already have module
> > > > > > > autoloading that works correctly for TPM devices, so why is
> > > > > > > this needed?
> > > > > > >
> > > > > > > We do have a chicken and egg problem with IMA in that the TPM
> > > > > > > driver needs to be present *before* any filesystem, including
> > > > > > > the one the TPM modules would be on, is mounted so executions
> > > > > > > can be measured into IMA (meaning that if you use IMA the TPM
> > > > > > > drivers must be built in) but this sounds to be something
> > > > > > > different. However, because of the IMA problem, most
> > > > > > > distributions don't end up compiling TPM drivers as modules
> > > > > > > anyway.
> > > > > > >
> > > > > > > Is what you want simply that tpm modules be loaded earlier?
> > > > > >
> > > > > > Yes, ealier is the problem. In my specific testing case the
> > > > > > machine is qemu arm64 with swtpm with EFI firmware for secure
> > > > > > boot and TPM support.
> > > > > >
> > > > > > Firmware uses TPM and does measurements and thus TPM event log
> > > > > > is
> > > > > > available on this machine and a bunch of other arm64 boards.
> > > > > > Visible in early boot dmesg as TPMEventLog lines like:
> > > > > >
> > > > > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > > > >
> > > > > > Different boards use different TPM HW and drivers so compiling
> > > > > > all these in is possible but a bit ugly. systemd recently
> > > > > > gained support for a specific tpm2.target which makes TPM
> > > > > > support modular and also works with kernel modules for some TPM
> > > > > > use cases but not rootfs encryption.
> > > > > >
> > > > > > In my test case we have a kernel+initramfs uki binary which is
> > > > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > > > various boards is visible in devicetree but not as ACPI table
> > > > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via
> > > > > > firmware measurement via
> > > > > > /sys/kernel/security/tpm0/binary_bios_measurements
> > > > > > .
> > > > >
> > > > > One corner case worth noting here is that scanning the device
> > > > > tree won't always work for non-ACPI systems... The reason is that
> > > > > a firmware TPM (running in OP-TEE) might or might not have a DT
> > > > > entry, since OP-TEE can discover the device dynamically and
> > > > > doesn't always rely on a DT entry.
> > > > >
> > > > > I don't particularly love the idea that an EventLog existence
> > > > > automatically means a TPM will be there, but it seems that
> > > > > systemd already relies on that and it does solve the problem we
> > > > > have.
> > > >
> > > > Well, quite. That's why the question I was interested in, perhaps
> > > > not asked as clearly as it could be is: since all the TPM devices
> > > > rely on discovery mechanisms like ACPI or DT or the like which are
> > > > ready quite early, should we simply be auto loading the TPM drivers
> > > > earlier?
> > >
> > > This would be an elegant way to solve this and on top of that, we
> > > have a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> > > But to answer that we need more feedback from systemd. What 'earlier'
> > > means? Autload it from the kernel before we go into launching the
> > > initrd?
> >
> > Right, so this is another timing problem: we can't autoload modules
> > *before* they appear in the filesystem and presumably they're on the
> > initrd, so auto loading must be post initrd mount (and init execution)
> > but otherwise quite early?
>
> Exactly. But is that enough?
>
> >
> > This might be quite a bit of work. Logically, just moving the matching
> > and loading code earlier might work, but we used to have a
> > load_default_modules() at the end of init/main.c and it got removed
> > (because it only eventually loaded elevator modules) everything is now
> > loaded in it's various init routines, so to get, say, TPM ACPI modules
> > loaded earlier, we'd have to run the ACPI device matching code earlier
> > and so on for every other subsystem ...
>
> Being the devil's advocate here and as I stated I don't love this but ...
> The kernel isn't technically doing anything wrong. We just expose an
> *existing* EFI config table. The kernel also exposes filesystems so
> people are free to do rm -rf *.
> The fact that applications might use it as a means of "oh there's
> probably a TPM" shouldn't be the end of the world. On top of that,
> since it's an EFI config table we can keep it around and never break
> any ABIs we create to userspace. If in the future we find a better
> way, userspace can use that?
>
> So perhaps this is ok as long as make sure we understand why systemd
> needs it that early?
>

What I would like to know is which API systemd is attempting to use,
and which -apparently- may never become available if no TPM is exposed
by the kernel.

Ideally, we should be able to take inspiration from the probe deferral
work, and return -EAGAIN to convey that it is too early to signal
either success or permanent failure.

Exposing random firmware assets directly to user space to make guesses
about this doesn't seem like a very robust approach to this issue.

2024-04-25 08:59:18

by Mikko Rapeli

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi,

On Wed, Apr 24, 2024 at 07:15:35PM +0200, Ard Biesheuvel wrote:
> On Mon, 22 Apr 2024 at 17:22, Ilias Apalodimas
> <[email protected]> wrote:
> >
> > On Mon, 22 Apr 2024 at 17:31, James Bottomley
> > <[email protected]> wrote:
> > >
> > > On Mon, 2024-04-22 at 16:54 +0300, Ilias Apalodimas wrote:
> > > > Hi James
> > > >
> > > > On Mon, 22 Apr 2024 at 16:38, James Bottomley
> > > > <[email protected]> wrote:
> > > > >
> > > > > On Mon, 2024-04-22 at 16:32 +0300, Ilias Apalodimas wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > On Mon, 22 Apr 2024 at 16:08, Mikko Rapeli
> > > > > > <[email protected]>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Mon, Apr 22, 2024 at 08:42:41AM -0400, James Bottomley
> > > > > > > wrote:
> > > > > > > > On Mon, 2024-04-22 at 14:27 +0300, Mikko Rapeli wrote:
> > > > > > > > > Userspace needs to know if TPM kernel drivers need to be
> > > > > > > > > loaded and related services started early in the boot if
> > > > > > > > > TPM device is used and available.
> > > > > > > >
> > > > > > > > This says what but not why. We already have module
> > > > > > > > autoloading that works correctly for TPM devices, so why is
> > > > > > > > this needed?
> > > > > > > >
> > > > > > > > We do have a chicken and egg problem with IMA in that the TPM
> > > > > > > > driver needs to be present *before* any filesystem, including
> > > > > > > > the one the TPM modules would be on, is mounted so executions
> > > > > > > > can be measured into IMA (meaning that if you use IMA the TPM
> > > > > > > > drivers must be built in) but this sounds to be something
> > > > > > > > different. However, because of the IMA problem, most
> > > > > > > > distributions don't end up compiling TPM drivers as modules
> > > > > > > > anyway.
> > > > > > > >
> > > > > > > > Is what you want simply that tpm modules be loaded earlier?
> > > > > > >
> > > > > > > Yes, ealier is the problem. In my specific testing case the
> > > > > > > machine is qemu arm64 with swtpm with EFI firmware for secure
> > > > > > > boot and TPM support.
> > > > > > >
> > > > > > > Firmware uses TPM and does measurements and thus TPM event log
> > > > > > > is
> > > > > > > available on this machine and a bunch of other arm64 boards.
> > > > > > > Visible in early boot dmesg as TPMEventLog lines like:
> > > > > > >
> > > > > > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > > > > >
> > > > > > > Different boards use different TPM HW and drivers so compiling
> > > > > > > all these in is possible but a bit ugly. systemd recently
> > > > > > > gained support for a specific tpm2.target which makes TPM
> > > > > > > support modular and also works with kernel modules for some TPM
> > > > > > > use cases but not rootfs encryption.
> > > > > > >
> > > > > > > In my test case we have a kernel+initramfs uki binary which is
> > > > > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > > > > various boards is visible in devicetree but not as ACPI table
> > > > > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via
> > > > > > > firmware measurement via
> > > > > > > /sys/kernel/security/tpm0/binary_bios_measurements
> > > > > > > .
> > > > > >
> > > > > > One corner case worth noting here is that scanning the device
> > > > > > tree won't always work for non-ACPI systems... The reason is that
> > > > > > a firmware TPM (running in OP-TEE) might or might not have a DT
> > > > > > entry, since OP-TEE can discover the device dynamically and
> > > > > > doesn't always rely on a DT entry.
> > > > > >
> > > > > > I don't particularly love the idea that an EventLog existence
> > > > > > automatically means a TPM will be there, but it seems that
> > > > > > systemd already relies on that and it does solve the problem we
> > > > > > have.
> > > > >
> > > > > Well, quite. That's why the question I was interested in, perhaps
> > > > > not asked as clearly as it could be is: since all the TPM devices
> > > > > rely on discovery mechanisms like ACPI or DT or the like which are
> > > > > ready quite early, should we simply be auto loading the TPM drivers
> > > > > earlier?
> > > >
> > > > This would be an elegant way to solve this and on top of that, we
> > > > have a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> > > > But to answer that we need more feedback from systemd. What 'earlier'
> > > > means? Autload it from the kernel before we go into launching the
> > > > initrd?
> > >
> > > Right, so this is another timing problem: we can't autoload modules
> > > *before* they appear in the filesystem and presumably they're on the
> > > initrd, so auto loading must be post initrd mount (and init execution)
> > > but otherwise quite early?
> >
> > Exactly. But is that enough?
> >
> > >
> > > This might be quite a bit of work. Logically, just moving the matching
> > > and loading code earlier might work, but we used to have a
> > > load_default_modules() at the end of init/main.c and it got removed
> > > (because it only eventually loaded elevator modules) everything is now
> > > loaded in it's various init routines, so to get, say, TPM ACPI modules
> > > loaded earlier, we'd have to run the ACPI device matching code earlier
> > > and so on for every other subsystem ...
> >
> > Being the devil's advocate here and as I stated I don't love this but ...
> > The kernel isn't technically doing anything wrong. We just expose an
> > *existing* EFI config table. The kernel also exposes filesystems so
> > people are free to do rm -rf *.
> > The fact that applications might use it as a means of "oh there's
> > probably a TPM" shouldn't be the end of the world. On top of that,
> > since it's an EFI config table we can keep it around and never break
> > any ABIs we create to userspace. If in the future we find a better
> > way, userspace can use that?
> >
> > So perhaps this is ok as long as make sure we understand why systemd
> > needs it that early?
> >
>
> What I would like to know is which API systemd is attempting to use,
> and which -apparently- may never become available if no TPM is exposed
> by the kernel.

ACPI TPM table entry, available without TPM drivers.

TPM PCR bios measurement, available only with working TPM drivers.

> Ideally, we should be able to take inspiration from the probe deferral
> work, and return -EAGAIN to convey that it is too early to signal
> either success or permanent failure.
>
> Exposing random firmware assets directly to user space to make guesses
> about this doesn't seem like a very robust approach to this issue.

As I understand, there are two questions which systemd and userspace need aswers to:

1) is there a TPM device

2) was a TPM device used by firmware to measure boot

If answer to 1) is yes, then drivers need to be loaded, but not necessarily
in early boot before main rootfs mount. If answer to 2) is yes, then
drivers and userspace components should be setup for TPM in early boot.

The x86 and server grade arm standard for 1) is ACPI table TPM entry, basically
/sys/firmware/acpi/tables/TPM2. For other HW, like embedded arm boards/SoCs,
the TPM device can be on discoverable bus (optee firmware TPM etc)
or in devicetree. These work correctly with udev etc and drivers get loaded
in normal boot and thus answer 1).

For 2), once kernel side TPM device has been initialized, driver built into the
kernel or loaded before this check, userspace and systemd can check if
/sys/firmware/acpi/tables/TPM2 is there, or alternatively if TPM driver
is already working /sys/kernel/security/tpm0/binary_bios_measurements
which indicates that firmware was measured with TPM. The latter really requires
built in TPM drivers or modules which are loaded really early in boot, almost
as first thing in userspace, before systemd does any checks for the TPM
device existance. These steps happen in initramfs so that TPM can be used
to protect main rootfs. systemd implements this with efi_has_tpm2()
function in https://github.com/systemd/systemd/blob/main/src/shared/efi-api.c#L482

One problem is how to support TPM devices as kernel modules. Linux distros
should be able to build these drivers too as modules and systemd etc will
take care to load them once devices are discovered, and then queue in
other possibly complex userspace services/daemons so that TPM is useful and
rootfs can be decrypted. /sys/kernel/security/tpm0/binary_bios_measurements is
available only when the TPM driver is loaded and working. For example, if boot firmware has
optee and firmware TPM (fTPM) as an early trusted application (early TA), then this
only works when optee driver is initialized, tee-supplicant userspace daemon runs
and tpm_ftpm_tee kernel driver for fTPM has been loaded to the kernel. tee-supplicant
implements RPMB storage if board/HW does not support it directly. The optee and fTPM
kernel drivers can be built into kernel, but they still need tee-supplicant in
userspace.

To answer 2), ACPI TPM table entry works without kernel drivers, but what
about devices without ACPI support, or TPM on a discoverable bus like optee with fTPM?

Parsing devicetree in userspace is really painful, but could possible be done.
Or kernel could provide a sysfs indicator if there is a devicetree entry
for TPM. But there are some devices where devicetree does not show a TPM
device but one exists in the discoverable bus, optee fTPM.

In all, granted limited, arm64 cases which I have seen, when firmare was aware
of the TPM device and used it for measuring boot, then the TPM Event Log was
provided to the kernel. The early boot log messages like show this:

arm64 qemu with swtpm, u-boot based EFI firmware, TPM Event Log and TPM
in devicetree, no ACPI:

https://ledge.validation.linaro.org/scheduler/job/85933
[ 0.000000] efi: TPMFinalLog=0xbd649040 RTPROP=0xbd647040 SMBIOS 3.0=0xbe6a9000 TPMEventLog=0xbd5b0040 INITRD=0xbd5af040 MEMRESERVE=0xbd5ae040
..
tpm_tis@0 {
tpm_event_log_size = <0x18f>;
compatible = "tcg,tpm-tis-mmio";
reg = <0x00 0x5000>;
tpm_event_log_addr = <0x00 0x40100000>;
};

rockpi4b with firmware TPM, u-boot based EFI firmware, TPM Event Log but
no TPM in devicetree, no ACPI:

https://ledge.validation.linaro.org/scheduler/job/85936
[ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040 RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb381040 INITRD=0xeb380040 RNG=0xe5baf040 MEMRESERVE=0xe5bae040

synquacer with firmware TPM and real TPM, u-boot based EFI firmware,
TPM Event Log using fTPM but only real but disabled TPM in devicetree, no ACPI:

https://ledge.validation.linaro.org/scheduler/job/85934
[ 0.000000] efi: ESRT=0xf9c90040 TPMFinalLog=0xf9c94040 RTPROP=0xf9c92040 SMBIOS=0xf9c8e000 TPMEventLog=0xf9bf7040 INITRD=0xf9bf6040 RNG=0xf9bf5040 MEMRESERVE=0xf9bf4040
..
tpm_tis@10000000 {
compatible = "socionext,synquacer-tpm-mmio";
status = "disabled";
reg = <0x00 0x10000000 0x00 0x5000>;
};

kv260/zyncmp-kria-starter with real TPM, u-boot based EFI firmware,
TPM Event Log using TPM but no devicetree, no ACPI:

https://ledge.validation.linaro.org/scheduler/job/85937
[ 0.000000] efi: ESRT=0x777f8040 TPMFinalLog=0x777fc040 RTPROP=0x777fa040 SMBIOS=0x777f6000 TPMEventLog=0x7775a040 INITRD=0x77759040 MEMRESERVE=0x77758040

Ampere AVA with ACPI TPM entry but support disabled in firmare,
no TPM Event Log, no devicetree, but an ACPI TPM entry:

https://ledge.validation.linaro.org/scheduler/job/85935
[ 0.000000] ACPI: TPM2 0x00000000FFFEFD18 00004C (v04 Ampere Altra 00000002 AMP. 01000013)

Some of these TPM indicators could be considered firmware bugs, but in all
cases TPM firmware support can be detected via TPM Event Log
so that answers question 2).

TPM Event Log is not a random firmware interface. It is an optional interface
to make TPM usable and in practice most TPM using firmware provide it to the kernel.
The actual measurement and checks for it require the TPM HW and measurement PCRs,
but the TPM Event Log is a strong indicator from firmware to the kernel, and userspace,
that TPM is available and was used.

Kernel already knows about TPM Event Log. The proper interface to read it is from
securityfs file binary_bios_measurements implemented by loaded TPM driver.
Existence of this measurement is know before TPM driver is loaded and this change
only exports that to sysfs so that userspace can queue the TPM driver loading
and userspace services/daemons in the early initramfs before main rootfs is available.

Hope this helps,

-Mikko

2024-04-25 09:59:16

by Lennart Poettering

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mi, 24.04.24 19:15, Ard Biesheuvel ([email protected]) wrote:

> > > > > > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > > > > >
> > > > > > > Different boards use different TPM HW and drivers so compiling
> > > > > > > all these in is possible but a bit ugly. systemd recently
> > > > > > > gained support for a specific tpm2.target which makes TPM
> > > > > > > support modular and also works with kernel modules for some TPM
> > > > > > > use cases but not rootfs encryption.
> > > > > > >
> > > > > > > In my test case we have a kernel+initramfs uki binary which is
> > > > > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > > > > various boards is visible in devicetree but not as ACPI table
> > > > > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via
> > > > > > > firmware measurement via
> > > > > > > /sys/kernel/security/tpm0/binary_bios_measurements
> > > > > > > .
> > > > > >
> > > > > > One corner case worth noting here is that scanning the device
> > > > > > tree won't always work for non-ACPI systems... The reason is that
> > > > > > a firmware TPM (running in OP-TEE) might or might not have a DT
> > > > > > entry, since OP-TEE can discover the device dynamically and
> > > > > > doesn't always rely on a DT entry.
> > > > > >
> > > > > > I don't particularly love the idea that an EventLog existence
> > > > > > automatically means a TPM will be there, but it seems that
> > > > > > systemd already relies on that and it does solve the problem we
> > > > > > have.
> > > > >
> > > > > Well, quite. That's why the question I was interested in, perhaps
> > > > > not asked as clearly as it could be is: since all the TPM devices
> > > > > rely on discovery mechanisms like ACPI or DT or the like which are
> > > > > ready quite early, should we simply be auto loading the TPM drivers
> > > > > earlier?
> > > >
> > > > This would be an elegant way to solve this and on top of that, we
> > > > have a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> > > > But to answer that we need more feedback from systemd. What 'earlier'
> > > > means? Autload it from the kernel before we go into launching the
> > > > initrd?
> > >
> > > Right, so this is another timing problem: we can't autoload modules
> > > *before* they appear in the filesystem and presumably they're on the
> > > initrd, so auto loading must be post initrd mount (and init execution)
> > > but otherwise quite early?
> >
> > Exactly. But is that enough?

General purpose distros typically don't build all TPM drivers into the
kernel, but ship some in the initrd instead. Then, udev is responsible
for iterating all buses/devices and auto-loading the necessary
drivers. Each loaded bus driver might make more devices available for
which more drivers then need to be loaded, and so on. Some of the
busses are "slow" in the sense that we don't really know a precise
time when we know that all devices have now shown up, there might
always be slow devices that haven't popped up yet. Iterating through
the entire tree of devices in sysfs is often quite slow in itself too,
it's one of the most time consuming parts of the boot in fact. This
all is done asynchronously hence: we enumerate/trigger/kmod all
devices as quickly as we can, but we continue doing other stuff at the
same time.

Of course that means that other stuff sometimes has to *wait* for
devices to show up. For example, if a harddisk shall be mounted, it
needs to be found/probed/kmod'ed first. Hence that's what we do: the
fscking/mounting of a file system is delayed exactly as long as it
takes for the block device it is for to show up.

systemd these days makes use of the TPM — if available — for various
purposes, such as disk encryption, measuring boot phases and system
identity and various other things. Now, for the purpose of disk
encryption, we need to wait for two things: the hard drive, and the
TPM to be probed/driver loaded/accessible. /etc/fstab tells us pretty
explicitly what bloock device to wait for, hence it's easy. But
waiting for a TPM is harder: we might need it for disk encryption, but
we don't know right-away if there actually *is* a TPM device to show
up, and hence don't know whether to wait for it or not.

In systemd we hence check early if firmware reported that it measured
something into a TPM during early boot. If not, then we have no
measured boot, and we don't have to wait for the TPM. (Of course,
there could possibly be a TPM that the firmware didn't use, but even
if, there's no real point in waiting for it, since a TPM is not that
interesting if your firmware didn't protect the boot with it/filled
the PCRs with data). If however we see that firmware measured
something into the TPM, then we deduce from that that Linux will probably
find the device too sooner or later, we just need to give the udev
enumerate/trigger/kmod logic enough time. And so we tell the disk
encryption stuff to wait for a TPM to show up before we continue. This
works quite well in the x86/acpi/uefi world.

There, we check for the efi tpm event log + the ACPI TPM table as
indication whether TPM was used by firmware. Note that we are not
interested in the *contents* of the log or the table to determine if
we shall wait for a tpm device, we just care about the
*existence*. i.e. it's two access() calls that check if the two
things exist in sysfs.

On ACPI-less arm platforms this check doesn't work. And we are looking
for a sensible replacement: i.e. something that tells us that the
firmware found, initialized, and measured stuff into a TPM and that is
enough for us to assume that sooner or later it will also be probed
and be accessible in Linux/udev/…, too.

This check is done very very early in userspace (early initrd), at a
moment in time where no devices have been probed yet.

How precisely we do the check is up for discussion. All we care about
is an answer to the question "Did firmware find and make use/measure
into a TPM device? yes/no". And we want to be able to answer that
question very early in userspace, without having to actually probe
hardware/busses/drivers/….

And yes, the test we implemented on x86/acpu/uefi might actually come
to the wrong conclusion, because Linux might lack a driver for a TPM
the firmware supports. But right now there are few reports this is
actually a problem, Linux TPM support seems to be quite OK
IRL. Moreover, there's an override via the kernel cmdline
(systemd.tpm2_wait=0) for the cases where the logic fails.

> What I would like to know is which API systemd is attempting to use,
> and which -apparently- may never become available if no TPM is exposed
> by the kernel.

We want a trivial check whether it's likely Linux is going to expose a
TPM device eventually.

And yes, as mentioned, there might be cases where Linux lacks a driver
for something the firmware support. But as long as this is not the
common case we are good.

> Ideally, we should be able to take inspiration from the probe deferral
> work, and return -EAGAIN to convey that it is too early to signal
> either success or permanent failure.

We need an early check, so that we can dealy FDE setup, that we can
delay our first boot phase measurements and so on. An kernel interface
that just tells us "we don't have a tpm yet, we might have one one
day, or not, we don't know yet" is useless to us.

> Exposing random firmware assets directly to user space to make guesses
> about this doesn't seem like a very robust approach to this issue.

If you give us a generic flag file that says "firmware found and used
a tpm" somewhere in sysfs that abstracts the details how it detects
that is enough for us. i.e. i don't care if the kenrel abstracts this
or if we do more explicit checks in userspace. All i care is that it's
just a few access(F_OK) checks away for us.

Lennart

--
Lennart Poettering, Berlin

2024-04-25 11:04:48

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi Lennart,

Thanks to you and Mikko for providing this background.

On Thu, 25 Apr 2024 at 11:59, Lennart Poettering <[email protected]> wrote:
>
> On Mi, 24.04.24 19:15, Ard Biesheuvel ([email protected]) wrote:
>
> > > > > > > > [ 0.000000] efi: ESRT=0xf0ea5040 TPMFinalLog=0xf0ea9040
> > > > > > > > RTPROP=0xf0ea7040 SMBIOS=0xf0ea3000 TPMEventLog=0xeb3b3040
> > > > > > > > INITRD=0xeb3b2040 RNG=0xe5c0f040 MEMRESERVE=0xe5c0e040
> > > > > > > >
> > > > > > > > Different boards use different TPM HW and drivers so compiling
> > > > > > > > all these in is possible but a bit ugly. systemd recently
> > > > > > > > gained support for a specific tpm2.target which makes TPM
> > > > > > > > support modular and also works with kernel modules for some TPM
> > > > > > > > use cases but not rootfs encryption.
> > > > > > > >
> > > > > > > > In my test case we have a kernel+initramfs uki binary which is
> > > > > > > > loaded by EFI firmware as a secure boot binary. TPM support on
> > > > > > > > various boards is visible in devicetree but not as ACPI table
> > > > > > > > entries. systemd currently detect TPM2 support either via ACPI
> > > > > > > > table /sys/firmware/acpi/tables/TPM2 or TPM entry or via
> > > > > > > > firmware measurement via
> > > > > > > > /sys/kernel/security/tpm0/binary_bios_measurements
> > > > > > > > .
> > > > > > >
> > > > > > > One corner case worth noting here is that scanning the device
> > > > > > > tree won't always work for non-ACPI systems... The reason is that
> > > > > > > a firmware TPM (running in OP-TEE) might or might not have a DT
> > > > > > > entry, since OP-TEE can discover the device dynamically and
> > > > > > > doesn't always rely on a DT entry.
> > > > > > >
> > > > > > > I don't particularly love the idea that an EventLog existence
> > > > > > > automatically means a TPM will be there, but it seems that
> > > > > > > systemd already relies on that and it does solve the problem we
> > > > > > > have.
> > > > > >
> > > > > > Well, quite. That's why the question I was interested in, perhaps
> > > > > > not asked as clearly as it could be is: since all the TPM devices
> > > > > > rely on discovery mechanisms like ACPI or DT or the like which are
> > > > > > ready quite early, should we simply be auto loading the TPM drivers
> > > > > > earlier?
> > > > >
> > > > > This would be an elegant way to solve this and on top of that, we
> > > > > have a single discovery mechanism from userspace -- e.g ls /dev/tpmX.
> > > > > But to answer that we need more feedback from systemd. What 'earlier'
> > > > > means? Autload it from the kernel before we go into launching the
> > > > > initrd?
> > > >
> > > > Right, so this is another timing problem: we can't autoload modules
> > > > *before* they appear in the filesystem and presumably they're on the
> > > > initrd, so auto loading must be post initrd mount (and init execution)
> > > > but otherwise quite early?
> > >
> > > Exactly. But is that enough?
>
> General purpose distros typically don't build all TPM drivers into the
> kernel, but ship some in the initrd instead. Then, udev is responsible
> for iterating all buses/devices and auto-loading the necessary
> drivers. Each loaded bus driver might make more devices available for
> which more drivers then need to be loaded, and so on. Some of the
> busses are "slow" in the sense that we don't really know a precise
> time when we know that all devices have now shown up, there might
> always be slow devices that haven't popped up yet. Iterating through
> the entire tree of devices in sysfs is often quite slow in itself too,
> it's one of the most time consuming parts of the boot in fact. This
> all is done asynchronously hence: we enumerate/trigger/kmod all
> devices as quickly as we can, but we continue doing other stuff at the
> same time.
>
> Of course that means that other stuff sometimes has to *wait* for
> devices to show up. For example, if a harddisk shall be mounted, it
> needs to be found/probed/kmod'ed first. Hence that's what we do: the
> fscking/mounting of a file system is delayed exactly as long as it
> takes for the block device it is for to show up.
>
> systemd these days makes use of the TPM — if available — for various
> purposes, such as disk encryption, measuring boot phases and system
> identity and various other things. Now, for the purpose of disk
> encryption, we need to wait for two things: the hard drive, and the
> TPM to be probed/driver loaded/accessible. /etc/fstab tells us pretty
> explicitly what bloock device to wait for, hence it's easy. But
> waiting for a TPM is harder: we might need it for disk encryption, but
> we don't know right-away if there actually *is* a TPM device to show
> up, and hence don't know whether to wait for it or not.
>

I take it this means that the LUKS metadata lacks a 'this key is
sealed into the TPM' bit?

Could you elaborate a bit on how the early boot code manages this?
..
> > Exposing random firmware assets directly to user space to make guesses
> > about this doesn't seem like a very robust approach to this issue.
>
> If you give us a generic flag file that says "firmware found and used
> a tpm" somewhere in sysfs that abstracts the details how it detects
> that is enough for us. i.e. i don't care if the kenrel abstracts this
> or if we do more explicit checks in userspace. All i care is that it's
> just a few access(F_OK) checks away for us.
>

So exposing the physical address of the TPM event log is probably not
what we want here.

Note that the TPM event log table is a Linux/efistub construct,
whereas the TPM final log table actually comes from the firmware
directly. So the former only exists if the EFI stub executed first,
and managed to invoke the TCG protocol etc. OTOH, the TPM final log is
TPM2 only, so it doesn't exist on TPM 1.2

Another thing we need to consider is TDX, which exposes a pseudo-TPM
which does not support sealing, along with a CC protocol similar to
the TCG2 protocol. This code will use the event log infrastructure as
well: there are discussions going on at the moment whether we can
improve the way these protocols are combined.

So we should define a scope here:
- do we need TPM1.2 support?
- do we need non-EFI boot support?
- do we need to do anything in particular for FDE on TDX, which has a
TPM event log but no TPM is likely to appear.

I am fine with adding a sysfs node under /sys/firmware/efi that
exposes some of this information, e.g.,
linux_efi_tpm_eventlog::version, but not the physical address of the
table.

2024-04-25 11:22:32

by Lennart Poettering

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Do, 25.04.24 12:36, Ard Biesheuvel ([email protected]) wrote:

> > systemd these days makes use of the TPM — if available — for various
> > purposes, such as disk encryption, measuring boot phases and system
> > identity and various other things. Now, for the purpose of disk
> > encryption, we need to wait for two things: the hard drive, and the
> > TPM to be probed/driver loaded/accessible. /etc/fstab tells us pretty
> > explicitly what bloock device to wait for, hence it's easy. But
> > waiting for a TPM is harder: we might need it for disk encryption, but
> > we don't know right-away if there actually *is* a TPM device to show
> > up, and hence don't know whether to wait for it or not.
> >
>
> I take it this means that the LUKS metadata lacks a 'this key is
> sealed into the TPM' bit?

Well, it does carry that info. But this is of no relevance
really. typically luks has multiple keys enrolled, and which slot(s) to
use is really up to the moment of invocation, depending on what is available.

moreover, i used disk encryption as one example, but we have more
users of TPM. for example we measure early in the initrd that we are
now in the initd, and when we leave the initrd we measure that we are
now gone. we also have a tool that sets up the TPM SRK. All that stuff
is supposed to be run if a TPM is available, but delayed just enough
until the TPM actually shows up, but no more. So for all of that stuff
we must have a way to delay tings correctly.

> So exposing the physical address of the TPM event log is probably not
> what we want here.
>
> Note that the TPM event log table is a Linux/efistub construct,
> whereas the TPM final log table actually comes from the firmware
> directly. So the former only exists if the EFI stub executed first,
> and managed to invoke the TCG protocol etc. OTOH, the TPM final log is
> TPM2 only, so it doesn't exist on TPM 1.2

(side note: in systemd we do not care about tpm 1.2 anymore, we only
support tpm2, and treat systems with just tpm 1.2 like systems that
have no tpm).

> Another thing we need to consider is TDX, which exposes a pseudo-TPM
> which does not support sealing, along with a CC protocol similar to
> the TCG2 protocol. This code will use the event log infrastructure as
> well: there are discussions going on at the moment whether we can
> improve the way these protocols are combined.

The way the delay for a tpm device is actually implemented in systemd
is somewhat generic: there's a target unit called
"tpm2.target". Typically we just delay its activation until a
/dev/tpmrm0 device shows up, if the firmware check suggested that. But
you could also stuff all kinds of other stuff before that. For exampe,
we could also delay it until some userspace service is running that
turns the local security tech into a "fake" tpm device or similar. So
from our side it's entirely pluggable already, and supports other ways
to synchronize properly on a TPM being available, people can plug in
whatever they need there for the synchronization.

However, our primary focus is to cover nicely the typical/generic
systems that have a discrete TPM or ftpm that just needs some generic
driver probing/loading to be available.

Or in other words: i am fully aware that a tpm-like device can be
provided by various means. For now, with this firmware flag file thing
we primarily care about the simple case where it's just driver loading
that we need to do.

> So we should define a scope here:
> - do we need TPM1.2 support?

For our systemd usecase, that's a clear no.

> - do we need non-EFI boot support?

My own focus is EFI. systemd supports non-EFI of course. If people
care about non-EFI I am sure they'd be thankful if we could provide a
similar automatism

> - do we need to do anything in particular for FDE on TDX, which has a
> TPM event log but no TPM is likely to appear.

I'd ignore that for now. And if they provide a software tpm-like device
eventually they just have to plug in the service that provides it
before tpm2.target and be done with it. I think we have an acceptable
approach for that already.

> I am fine with adding a sysfs node under /sys/firmware/efi that
> exposes some of this information, e.g.,
> linux_efi_tpm_eventlog::version, but not the physical address of the
> table.

Yeah, the physical address is of no interest to us. We just need to
know the existance, and that independently of any actualy tpm device
having shown up. i.e. existance of
/sys/kernel/security/tpm0/binary_bios_measurements would be good
enough for is if it was available without "tpm0" actually being
around...

Lennart

--
Lennart Poettering, Berlin

2024-04-25 11:47:58

by Ilias Apalodimas

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi all

On Thu, 25 Apr 2024 at 14:13, Lennart Poettering <[email protected]> wrote:
>
> On Do, 25.04.24 12:36, Ard Biesheuvel ([email protected]) wrote:
>
> > > systemd these days makes use of the TPM — if available — for various
> > > purposes, such as disk encryption, measuring boot phases and system
> > > identity and various other things. Now, for the purpose of disk
> > > encryption, we need to wait for two things: the hard drive, and the
> > > TPM to be probed/driver loaded/accessible. /etc/fstab tells us pretty
> > > explicitly what bloock device to wait for, hence it's easy. But
> > > waiting for a TPM is harder: we might need it for disk encryption, but
> > > we don't know right-away if there actually *is* a TPM device to show
> > > up, and hence don't know whether to wait for it or not.
> > >
> >
> > I take it this means that the LUKS metadata lacks a 'this key is
> > sealed into the TPM' bit?
>
> Well, it does carry that info. But this is of no relevance
> really. typically luks has multiple keys enrolled, and which slot(s) to
> use is really up to the moment of invocation, depending on what is available.
>
> moreover, i used disk encryption as one example, but we have more
> users of TPM. for example we measure early in the initrd that we are
> now in the initd, and when we leave the initrd we measure that we are
> now gone. we also have a tool that sets up the TPM SRK. All that stuff
> is supposed to be run if a TPM is available, but delayed just enough
> until the TPM actually shows up, but no more. So for all of that stuff
> we must have a way to delay tings correctly.
>
> > So exposing the physical address of the TPM event log is probably not
> > what we want here.
> >
> > Note that the TPM event log table is a Linux/efistub construct,
> > whereas the TPM final log table actually comes from the firmware
> > directly. So the former only exists if the EFI stub executed first,
> > and managed to invoke the TCG protocol etc. OTOH, the TPM final log is
> > TPM2 only, so it doesn't exist on TPM 1.2
>
> (side note: in systemd we do not care about tpm 1.2 anymore, we only
> support tpm2, and treat systems with just tpm 1.2 like systems that
> have no tpm).
>
> > Another thing we need to consider is TDX, which exposes a pseudo-TPM
> > which does not support sealing, along with a CC protocol similar to
> > the TCG2 protocol. This code will use the event log infrastructure as
> > well: there are discussions going on at the moment whether we can
> > improve the way these protocols are combined.
>
> The way the delay for a tpm device is actually implemented in systemd
> is somewhat generic: there's a target unit called
> "tpm2.target". Typically we just delay its activation until a
> /dev/tpmrm0 device shows up, if the firmware check suggested that. But
> you could also stuff all kinds of other stuff before that. For exampe,
> we could also delay it until some userspace service is running that
> turns the local security tech into a "fake" tpm device or similar. So
> from our side it's entirely pluggable already, and supports other ways
> to synchronize properly on a TPM being available, people can plug in
> whatever they need there for the synchronization.
>
> However, our primary focus is to cover nicely the typical/generic
> systems that have a discrete TPM or ftpm that just needs some generic
> driver probing/loading to be available.
>
> Or in other words: i am fully aware that a tpm-like device can be
> provided by various means. For now, with this firmware flag file thing
> we primarily care about the simple case where it's just driver loading
> that we need to do.
>
> > So we should define a scope here:
> > - do we need TPM1.2 support?
>
> For our systemd usecase, that's a clear no.
>
> > - do we need non-EFI boot support?
>
> My own focus is EFI. systemd supports non-EFI of course. If people
> care about non-EFI I am sure they'd be thankful if we could provide a
> similar automatism
>
> > - do we need to do anything in particular for FDE on TDX, which has a
> > TPM event log but no TPM is likely to appear.
>
> I'd ignore that for now. And if they provide a software tpm-like device
> eventually they just have to plug in the service that provides it
> before tpm2.target and be done with it. I think we have an acceptable
> approach for that already.
>
> > I am fine with adding a sysfs node under /sys/firmware/efi that
> > exposes some of this information, e.g.,
> > linux_efi_tpm_eventlog::version, but not the physical address of the
> > table.
>
> Yeah, the physical address is of no interest to us. We just need to
> know the existance, and that independently of any actualy tpm device
> having shown up. i.e. existance of
> /sys/kernel/security/tpm0/binary_bios_measurements would be good
> enough for is if it was available without "tpm0" actually being
> around...

IIRC 'binary_bios_measurements' is only created after the TPM drivers
probe the device, so that wouldn't work.
Ard is right though the TPMEventLog is an EFI stub construct, so
exposing this is Linux-specific (and stub-specific).
The TPMFinalLog OTOH is described by the TCG spec so exposing that
even using the address address would work for systemd

Regards
/Ilias


>
> Lennart
>
> --
> Lennart Poettering, Berlin

2024-04-25 13:36:43

by Lennart Poettering

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Do, 25.04.24 14:47, Ilias Apalodimas ([email protected]) wrote:

> > Yeah, the physical address is of no interest to us. We just need to
> > know the existance, and that independently of any actualy tpm device
> > having shown up. i.e. existance of
> > /sys/kernel/security/tpm0/binary_bios_measurements would be good
> > enough for is if it was available without "tpm0" actually being
> > around...
>
> IIRC 'binary_bios_measurements' is only created after the TPM drivers
> probe the device, so that wouldn't work.
> Ard is right though the TPMEventLog is an EFI stub construct, so
> exposing this is Linux-specific (and stub-specific).
> The TPMFinalLog OTOH is described by the TCG spec so exposing that
> even using the address address would work for systemd

Hmm, let me ask explicitly: is there any good reason for
'binary_bios_measurements' being tied to specific TPM devices? i mean
it just exposes some firmware-provided memory area, no?

So, if the answer to that question is "no", maybe we can just move the
file to some generic place that is not tied to "tpm0" being around,
and then make the current file a symlink to that new place for compat?

i.e. /sys/kernel/security/tpm0/binary_bios_measurements could be a
symlink to → /sys/kernel/security/binary_bios_measurements and the
latter could be something the kernel always exposes, before any tpm
drivers are loaded?

Lennart

--
Lennart Poettering, Berlin

2024-04-25 13:40:09

by Mikko Rapeli

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi,

On Thu, Apr 25, 2024 at 09:24:48AM -0400, James Bottomley wrote:
> On Thu, 2024-04-25 at 11:58 +0200, Lennart Poettering wrote:
> [...]
> > General purpose distros typically don't build all TPM drivers into
> > the kernel, but ship some in the initrd instead. Then, udev is
> > responsible for iterating all buses/devices and auto-loading the
> > necessary drivers. Each loaded bus driver might make more devices
> > available for which more drivers then need to be loaded, and so on.
> > Some of the busses are "slow" in the sense that we don't really know
> > a precise time when we know that all devices have now shown up, there
> > might always be slow devices that haven't popped up yet. Iterating
> > through the entire tree of devices in sysfs is often quite slow in
> > itself too, it's one of the most time consuming parts of the boot in
> > fact. This all is done asynchronously hence: we
> > enumerate/trigger/kmod all devices as quickly as we can, but we
> > continue doing other stuff at the same time.
>
> So let me make a suggestion that you can use now. Since all you
> currently care about is the EFI/ACPI device, there is always a single
> sysfs entry that corresponds to that (so you shouldn't need the log
> entry as an indicator):
>
> /sys/bus/acpi/devices/MSFT0101\:00
>
> That link (or a kobject uevent if you prefer to look for that) will
> always appear regardless of whether a driver has attached or not. When
> the driver actually attaches, a driver/ directory will appear where the
> link points.
>
> The device link is added when the acpi scan is initiated as a
> subsys_initcall, which is before all the filesystem initcalls, so it
> should run before the initrd is mounted.
>
> Is this enough for now and we can think about a more generic indicator
> that all drivers have been probed later?

This covers EFI ACPI devices but not devices without ACPI.

Some boards have the TPM device data in devicetree.

Some boards have a firmware TPM which is not listed in devicetree
but is detected at runtime once optee drivers, tee-supplicant, etc have
loaded.

Based on the comments here, I could propose a v2 with TPMFinalLog based
sysfs file which is empty but existence of the file shows to systemd that
EFI firmware used a TPM device and that can queue in the normal ACPI,
devicetree or other mechanisms of detecting the real TPM device, loading drivers
and possibly needed userspace components like tee-supplicant for optee and fTPM.

I don't know how non-EFI firmware could be supported, unless they show the ACPI TPM
entry.

Cheers,

-Mikko

2024-04-25 13:40:27

by Lennart Poettering

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Do, 25.04.24 09:24, James Bottomley ([email protected]) wrote:

> On Thu, 2024-04-25 at 11:58 +0200, Lennart Poettering wrote:
> [...]
> > General purpose distros typically don't build all TPM drivers into
> > the kernel, but ship some in the initrd instead. Then, udev is
> > responsible for iterating all buses/devices and auto-loading the
> > necessary drivers. Each loaded bus driver might make more devices
> > available for which more drivers then need to be loaded, and so on.
> > Some of the busses are "slow" in the sense that we don't really know
> > a precise time when we know that all devices have now shown up, there
> > might always be slow devices that haven't popped up yet. Iterating
> > through the entire tree of devices in sysfs is often quite slow in
> > itself too, it's one of the most time consuming parts of the boot in
> > fact. This all is done asynchronously hence: we
> > enumerate/trigger/kmod all devices as quickly as we can, but we
> > continue doing other stuff at the same time.
>
> So let me make a suggestion that you can use now. Since all you
> currently care about is the EFI/ACPI device, there is always a single
> sysfs entry that corresponds to that (so you shouldn't need the log
> entry as an indicator):
>
> /sys/bus/acpi/devices/MSFT0101\:00
>
> That link (or a kobject uevent if you prefer to look for that) will
> always appear regardless of whether a driver has attached or not. When
> the driver actually attaches, a driver/ directory will appear where the
> link points.
>
> The device link is added when the acpi scan is initiated as a
> subsys_initcall, which is before all the filesystem initcalls, so it
> should run before the initrd is mounted.
>
> Is this enough for now and we can think about a more generic indicator
> that all drivers have been probed later?

That would only work on ACPI though, but on ACPI we already have a
check that works?

Or to say this differently: how is that different/better from the
check that already exists in systemd, which looks for
/sys/firmware/acpi/tables/TPM2?

Lennart

--
Lennart Poettering, Berlin

2024-04-25 13:43:44

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Thu, 2024-04-25 at 11:58 +0200, Lennart Poettering wrote:
[...]
> General purpose distros typically don't build all TPM drivers into
> the kernel, but ship some in the initrd instead. Then, udev is
> responsible for iterating all buses/devices and auto-loading the
> necessary drivers. Each loaded bus driver might make more devices
> available for which more drivers then need to be loaded, and so on.
> Some of the busses are "slow" in the sense that we don't really know
> a precise time when we know that all devices have now shown up, there
> might always be slow devices that haven't popped up yet. Iterating
> through the entire tree of devices in sysfs is often quite slow in
> itself too, it's one of the most time consuming parts of the boot in
> fact. This all is done asynchronously hence: we
> enumerate/trigger/kmod all devices as quickly as we can, but we
> continue doing other stuff at the same time.

So let me make a suggestion that you can use now. Since all you
currently care about is the EFI/ACPI device, there is always a single
sysfs entry that corresponds to that (so you shouldn't need the log
entry as an indicator):

/sys/bus/acpi/devices/MSFT0101\:00

That link (or a kobject uevent if you prefer to look for that) will
always appear regardless of whether a driver has attached or not. When
the driver actually attaches, a driver/ directory will appear where the
link points.

The device link is added when the acpi scan is initiated as a
subsys_initcall, which is before all the filesystem initcalls, so it
should run before the initrd is mounted.

Is this enough for now and we can think about a more generic indicator
that all drivers have been probed later?

James


2024-04-25 13:50:27

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Thu Apr 25, 2024 at 11:56 AM EEST, Mikko Rapeli wrote:
> 1) is there a TPM device

Translates to "Does /sys/class/tpm/tpm0 exists?"

TPM version can be determined with
/sys/class/tpm/tpm0/tpm_version_major

BR, Jarkko

2024-04-25 14:01:42

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> General purpose distros typically don't build all TPM drivers into the
> kernel, but ship some in the initrd instead. Then, udev is responsible
> for iterating all buses/devices and auto-loading the necessary
> drivers. Each loaded bus driver might make more devices available for

I've had since day 0 that I've worked with TPM driver (i.e. since 2013
or 2014) that module support should be removed.

I've kept the module compilation only because huge turnback from the
community.

It does not make sense:

1. Because it makes sense as part of "TCB".
2. "TCB" is should in be vmlinux.
3. TPM is also a subsystem with other clients in the kernel.

At minimum the main TPM driver should IMHO just in vmlinux e.g. because
it is rare to see distro kernel with TPM enabled and IMA disabled, I
don't know any.

That said, I would not mind either if TPM subsystem drivers were only
y/n *except* tpm_vtpm_proxy.

BR, Jarkko

2024-04-25 14:06:40

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Thu, 2024-04-25 at 15:36 +0200, Lennart Poettering wrote:
> On Do, 25.04.24 14:47, Ilias Apalodimas ([email protected])
> wrote:
>
> > > Yeah, the physical address is of no interest to us. We just need
> > > to
> > > know the existance, and that independently of any actualy tpm
> > > device
> > > having shown up. i.e. existance of
> > > /sys/kernel/security/tpm0/binary_bios_measurements would be good
> > > enough for is if it was available without "tpm0" actually being
> > > around...
> >
> > IIRC 'binary_bios_measurements' is only created after the TPM
> > drivers
> > probe the device, so that wouldn't work.
> > Ard is right though the TPMEventLog is an EFI stub construct, so
> > exposing this is Linux-specific (and stub-specific).
> > The TPMFinalLog OTOH is described by the TCG spec so exposing that
> > even using the address address would work for systemd
>
> Hmm, let me ask explicitly: is there any good reason for
> 'binary_bios_measurements' being tied to specific TPM devices? i mean
> it just exposes some firmware-provided memory area, no?

Realistically, no. The current mechanism for exposing it in securityfs
is tpm chip init, which means it can't appear before a TPM driver
attaches, but there's no real reason why that has to be so.

> So, if the answer to that question is "no", maybe we can just move
> the file to some generic place that is not tied to "tpm0" being
> around, and then make the current file a symlink to that new place
> for compat?

We could just keep it where it is. I don't believe a log ever appears
at anything other than tpm0 because the event log securityfs attach is
triggered on first tpm appearance, which has to be 0. So the name is
purely conventional and could be hard coded. The eventlog code itself
is all linked to the actual chip device (including some of the checks
for event log type---we have to work with both 1.2 and 2.0), but it
should be possible to get the information another way.

James


> i.e. /sys/kernel/security/tpm0/binary_bios_measurements could be a
> symlink to → /sys/kernel/security/binary_bios_measurements and the
> latter could be something the kernel always exposes, before any tpm
> drivers are loaded?





2024-04-26 07:36:05

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > General purpose distros typically don't build all TPM drivers into the
> > kernel, but ship some in the initrd instead. Then, udev is responsible
> > for iterating all buses/devices and auto-loading the necessary
> > drivers. Each loaded bus driver might make more devices available for
>
> I've had since day 0 that I've worked with TPM driver (i.e. since 2013

- had the opinion (typo)

BR, Jarkko

2024-04-26 07:41:54

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Fri Apr 26, 2024 at 10:35 AM EEST, Jarkko Sakkinen wrote:
> On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> > On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > > General purpose distros typically don't build all TPM drivers into the
> > > kernel, but ship some in the initrd instead. Then, udev is responsible
> > > for iterating all buses/devices and auto-loading the necessary
> > > drivers. Each loaded bus driver might make more devices available for
> >
> > I've had since day 0 that I've worked with TPM driver (i.e. since 2013
>
> - had the opinion (typo)

Tbh, I have zero idea what this discussion is about anyway because the
original thread *was not* CC'd to linux-integrity and I'm not subscribed
to linux-efi. So next time put all the relevant mailing lists. I.e.
definitive NAK for this patch.

BR, Jarkko

2024-04-26 08:20:02

by Mikko Rapeli

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

Hi,

On Fri, Apr 26, 2024 at 10:40:20AM +0300, Jarkko Sakkinen wrote:
> On Fri Apr 26, 2024 at 10:35 AM EEST, Jarkko Sakkinen wrote:
> > On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> > > On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > > > General purpose distros typically don't build all TPM drivers into the
> > > > kernel, but ship some in the initrd instead. Then, udev is responsible
> > > > for iterating all buses/devices and auto-loading the necessary
> > > > drivers. Each loaded bus driver might make more devices available for
> > >
> > > I've had since day 0 that I've worked with TPM driver (i.e. since 2013
> >
> > - had the opinion (typo)
>
> Tbh, I have zero idea what this discussion is about anyway because the
> original thread *was not* CC'd to linux-integrity and I'm not subscribed
> to linux-efi. So next time put all the relevant mailing lists. I.e.
> definitive NAK for this patch.

Sorry for not including linux-integrity. I added maintainers and lists
proposed by scripts/get_maintainers.pl for the change which did not touch
drivers/char/tpm/ though TPM event log APIs are clearly there.

The full thread starts from here:

https://lore.kernel.org/all/[email protected]/T/#u

Cheers,

-Mikko

2024-04-26 08:23:18

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Fri Apr 26, 2024 at 11:19 AM EEST, Mikko Rapeli wrote:
> Hi,
>
> On Fri, Apr 26, 2024 at 10:40:20AM +0300, Jarkko Sakkinen wrote:
> > On Fri Apr 26, 2024 at 10:35 AM EEST, Jarkko Sakkinen wrote:
> > > On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> > > > On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > > > > General purpose distros typically don't build all TPM drivers into the
> > > > > kernel, but ship some in the initrd instead. Then, udev is responsible
> > > > > for iterating all buses/devices and auto-loading the necessary
> > > > > drivers. Each loaded bus driver might make more devices available for
> > > >
> > > > I've had since day 0 that I've worked with TPM driver (i.e. since 2013
> > >
> > > - had the opinion (typo)
> >
> > Tbh, I have zero idea what this discussion is about anyway because the
> > original thread *was not* CC'd to linux-integrity and I'm not subscribed
> > to linux-efi. So next time put all the relevant mailing lists. I.e.
> > definitive NAK for this patch.
>
> Sorry for not including linux-integrity. I added maintainers and lists
> proposed by scripts/get_maintainers.pl for the change which did not touch
> drivers/char/tpm/ though TPM event log APIs are clearly there.
>
> The full thread starts from here:
>
> https://lore.kernel.org/all/[email protected]/T/#u

NP, just add CC to the next version.

I'll download it and check through once bandwidth.

> Cheers,
>
> -Mikko

BR, Jarkko

2024-04-26 11:42:15

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon Apr 22, 2024 at 2:27 PM EEST, Mikko Rapeli wrote:
> Userspace needs to know if TPM kernel drivers need to be loaded
> and related services started early in the boot if TPM device
> is used and available. If EFI firmware has used TPM device
> to e.g. measure binaries, then many of them also provide the TPM

What are the other uses cases? TPM settings and reset (clear), i.e.
machine owner use cases? I think "e.g." is not needed here and it
confuses a bit.

> log to kernel in addition to the actual TPM device side measurements.
> Expose availability of TPM event log to userspace via
> /sys/firmware/efi/tpm_log. If the file exists, then firmware
> provided a TPM event log to kernel, and userspace init should also
> queue TPM module loading and other early boot services for TPM support.
>
> Enables systemd to support TPM drivers as modules when rootfs is
> encrypted with the TPM device.

"Enabling systemd" is not an unambiguous sequence of events, as far
as I know.

Please describe what the changes are done to the kernel, and how they
help to enable whatever systemd wants it. This is way too abstract to
work as "a pitch".

>
> Sample output from a arm64 qemu machine with u-boot based EFI firmware
> and swtpm:
>
> root@trs-qemuarm64:~# dmesg|grep TPMEvent
> [ 0.000000] efi: TPMFinalLog=0xbd648040 RTPROP=0xbd646040 SMBIOS3.0=0xbe6ad000 TPMEventLog=0xbd5f9040 INITRD=0xbd5f7040 RNG=0xbd5f6040 MEMRESERVE=0xbd5f5040
> root@trs-qemuarm64:~# ls -l /sys/firmware/efi/tpm_log
> -r-------- 1 root root 4096 Apr 22 10:31 /sys/firmware/efi/tpm_log
> root@trs-qemuarm64:~# cat /sys/firmware/efi/tpm_log
> TPMEventLog=0xbd5f9040
> root@trs-qemuarm64:~# cat /sys/firmware/efi/systab
> SMBIOS3=0xbe6ad000
>
> Other similar information is currently in /sys/firmware/efi/systab but
> for new exported variables a one-variable-per-file sysfs interface
> is preferred according to comments in systab_show()
> drivers/firmware/efi/efi.c
>
> See also:
> https://github.com/systemd/systemd/pull/32314
> https://lists.freedesktop.org/archives/systemd-devel/2024-April/050206.html
>
> Cc: Ilias Apalodimas <[email protected]>
> Cc: Lennart Poettering <[email protected]>
> Signed-off-by: Mikko Rapeli <[email protected]>

I'd recommend to test this also with hardware. Easy options for ARM
are:

1. Raspberry Pi 3B+. It has broken TrustZone that allows supply your
own payloads. OP-TEE supports this.
2. Get https://thepihut.com/products/letstrust-tpm-for-raspberry-pi.

BR, Jarkko

2024-04-26 11:48:51

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] efi: expose TPM event log to userspace via sysfs

On Mon Apr 22, 2024 at 2:27 PM EEST, Mikko Rapeli wrote:
> Documentation/ABI/testing/sysfs-firmware-efi | 12 ++++++++++++
> drivers/firmware/efi/efi.c | 13 +++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi
> index 5e4d0b27cdfe..caaff27cc73e 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-efi
> +++ b/Documentation/ABI/testing/sysfs-firmware-efi
> @@ -36,3 +36,15 @@ Description: Displays the content of the Runtime Configuration Interface
> Table version 2 on Dell EMC PowerEdge systems in binary format
> Users: It is used by Dell EMC OpenManage Server Administrator tool to
> populate BIOS setup page.
> +
> +What: /sys/firmware/efi/tpm_log

*Conditional* suggestion: s/tpm_log/tpm_event_log/

I.e. if we want this better to use the most popular name.

> +Date: April 2024
> +Contact: Mikko Rapeli <[email protected]>
> +Description: If EFI firmware supports TPM device and measurements were done
> + then a TPM event log has very likely been generated and provided
> + to the kernel. This serves as indicator for userspace to load
> + TPM drivers and to start related service early in the boot sequence,
> + e.g. initramfs, where full bus probes and device scans are not yet
> + done.
> +Users: systemd will use this interface to support TPM drivers as modules also
> + for early initramfs
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 4fcda50acfa4..94773e8b8806 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -162,6 +162,13 @@ static ssize_t fw_platform_size_show(struct kobject *kobj,
> return sprintf(buf, "%d\n", efi_enabled(EFI_64BIT) ? 64 : 32);
> }
>
> +static ssize_t tpm_log_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "TPMEventLog=0x%lx", efi.tpm_log);

It is also asymmetric with the log message as we can see here. Why you
want to put that prefix and not just print the number? Why hex and not
just %lu?


> +}
> +static struct kobj_attribute efi_attr_tpm_log = __ATTR_RO_MODE(tpm_log, 0400);
> +
> extern __weak struct kobj_attribute efi_attr_fw_vendor;
> extern __weak struct kobj_attribute efi_attr_runtime;
> extern __weak struct kobj_attribute efi_attr_config_table;
> @@ -459,6 +466,12 @@ static int __init efisubsys_init(void)
> platform_device_register_simple("efi_secret", 0, NULL, 0);
> #endif
>
> + if (efi.tpm_log != EFI_INVALID_TABLE_ADDR) {
> + error = sysfs_create_file(efi_kobj, &efi_attr_tpm_log.attr);
> + if (error)
> + pr_err("sysfs create file failed with error %d.\n", error);
> + }

s/err/warn/

Why "sys create file" and not "sys_create_file"?

> +
> return 0;
>
> err_remove_group:

BR, Jarkko