This series introduces the tee based EFI Runtime Variable Service.
The eMMC device is typically owned by the non-secure world(linux in
this case). There is an existing solution utilizing eMMC RPMB partition
for EFI Variables, it is implemented by interacting with
OP-TEE, StandaloneMM(as EFI Variable Service Pseudo TA), eMMC driver
and tee-supplicant. The last piece is the tee-based variable access
driver to interact with OP-TEE and StandaloneMM.
Changelog:
v7 -> v8
Only patch #3 "efi: Add tee-based EFI variable driver" is updated.
- fix typos
- refactor error handling, direct return if applicable
- use devm_add_action_or_reset() for closing of tee context/session
- remove obvious comment
v6 -> v7
Patch #1-#4 are not updated.
Patch #5 is added into this series, original patch is here:
https://lore.kernel.org/all/[email protected]/
There are two issues in the v6 series and v7 series addresses those.
1) efivar ops is not restored when the tee-supplicant daemon terminates.
-> As the following patch says, user must remove the device before
terminating tee-supplicant daemon.
https://lore.kernel.org/all/[email protected]/
2) cause panic when someone remounts the efivarfs as RW even if
SetVariable is not supported
-> The fifth patch addresses this issue.
"[PATCH v7 5/5] efivarfs: force RO when remounting if SetVariable is
not supported"
v5 -> v6
- new patch #4 is added in this series, #1-#3 patches are unchanged.
automatically update super block flag when the efivarops support
SetVariable runtime service, so that user does not need to manually
remount the efivarfs as RW.
v4 -> v5
- rebase to efi-next based on v6.4-rc1
- set generic_ops.query_variable_info, it works as expected as follows.
$ df -h /sys/firmware/efi/efivars/
Filesystem Size Used Avail Use% Mounted on
efivarfs 16K 1.3K 15K 8% /sys/firmware/efi/efivars
v3 -> v4:
- replace the reference from EDK2 to PI Specification
- remove EDK2 source code reference comments
- prepare nonblocking variant of set_variable, it just returns
EFI_UNSUPPORTED
- remove redundant buffer size check
- argument name change in mm_communicate
- function interface changes in setup_mm_hdr to remove (void **) cast
v2 -> v3:
- add CONFIG_EFI dependency to TEE_STMM_EFI
- add missing return code check for tee_client_invoke_func()
- directly call efivars_register/unregister from tee_stmm_efi.c
rfc v1 -> v2:
- split patch into three patches, one for drivers/tee,
one for include/linux/efi.h, and one for the driver/firmware/efi/stmm
- context/session management into probe() and remove() same as other tee
client driver
- StMM variable driver is moved from driver/tee/optee to driver/firmware/efi
- use "tee" prefix instead of "optee" in driver/firmware/efi/stmm/tee_stmm_efi.c,
this file does not contain op-tee specific code, abstracted by tee layer and
StMM variable driver will work on other tee implementation.
- PTA_STMM_CMD_COMMUNICATE -> PTA_STMM_CMD_COMMUNICATE
- implement query_variable_store() but currently not used
- no use of TEEC_SUCCESS, it is defined in driver/tee/optee/optee_private.h.
Other tee client drivers use 0 instead of using TEEC_SUCCESS
- remove TEEC_ERROR_EXCESS_DATA status, it is referred just to output
error message
Ilias Apalodimas (1):
efivarfs: force RO when remounting if SetVariable is not supported
Masahisa Kojima (4):
efi: expose efivar generic ops register function
efi: Add EFI_ACCESS_DENIED status code
efi: Add tee-based EFI variable driver
efivarfs: automatically update super block flag
drivers/firmware/efi/Kconfig | 15 +
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/efi.c | 18 +
drivers/firmware/efi/stmm/mm_communication.h | 236 +++++++
drivers/firmware/efi/stmm/tee_stmm_efi.c | 612 +++++++++++++++++++
drivers/firmware/efi/vars.c | 8 +
fs/efivarfs/super.c | 45 ++
include/linux/efi.h | 12 +
8 files changed, 947 insertions(+)
create mode 100644 drivers/firmware/efi/stmm/mm_communication.h
create mode 100644 drivers/firmware/efi/stmm/tee_stmm_efi.c
base-commit: f6e6e95ce16205025b7b8680a66c30a0c4ec2270
--
2.30.2
From: Ilias Apalodimas <[email protected]>
If SetVariable at runtime is not supported by the firmware we never assign
a callback for that function. At the same time mount the efivarfs as
RO so no one can call that. However, we never check the permission flags
when someone remounts the filesystem as RW. As a result this leads to a
crash looking like this:
$ mount -o remount,rw /sys/firmware/efi/efivars
$ efi-updatevar -f PK.auth PK
[ 303.279166] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 303.280482] Mem abort info:
[ 303.280854] ESR = 0x0000000086000004
[ 303.281338] EC = 0x21: IABT (current EL), IL = 32 bits
[ 303.282016] SET = 0, FnV = 0
[ 303.282414] EA = 0, S1PTW = 0
[ 303.282821] FSC = 0x04: level 0 translation fault
[ 303.283771] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004258c000
[ 303.284913] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[ 303.286076] Internal error: Oops: 0000000086000004 [#1] PREEMPT SMP
[ 303.286936] Modules linked in: qrtr tpm_tis tpm_tis_core crct10dif_ce arm_smccc_trng rng_core drm fuse ip_tables x_tables ipv6
[ 303.288586] CPU: 1 PID: 755 Comm: efi-updatevar Not tainted 6.3.0-rc1-00108-gc7d0c4695c68 #1
[ 303.289748] Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2023.04-00627-g88336918701d 04/01/2023
[ 303.291150] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 303.292123] pc : 0x0
[ 303.292443] lr : efivar_set_variable_locked+0x74/0xec
[ 303.293156] sp : ffff800008673c10
[ 303.293619] x29: ffff800008673c10 x28: ffff0000037e8000 x27: 0000000000000000
[ 303.294592] x26: 0000000000000800 x25: ffff000002467400 x24: 0000000000000027
[ 303.295572] x23: ffffd49ea9832000 x22: ffff0000020c9800 x21: ffff000002467000
[ 303.296566] x20: 0000000000000001 x19: 00000000000007fc x18: 0000000000000000
[ 303.297531] x17: 0000000000000000 x16: 0000000000000000 x15: 0000aaaac807ab54
[ 303.298495] x14: ed37489f673633c0 x13: 71c45c606de13f80 x12: 47464259e219acf4
[ 303.299453] x11: ffff000002af7b01 x10: 0000000000000003 x9 : 0000000000000002
[ 303.300431] x8 : 0000000000000010 x7 : ffffd49ea8973230 x6 : 0000000000a85201
[ 303.301412] x5 : 0000000000000000 x4 : ffff0000020c9800 x3 : 00000000000007fc
[ 303.302370] x2 : 0000000000000027 x1 : ffff000002467400 x0 : ffff000002467000
[ 303.303341] Call trace:
[ 303.303679] 0x0
[ 303.303938] efivar_entry_set_get_size+0x98/0x16c
[ 303.304585] efivarfs_file_write+0xd0/0x1a4
[ 303.305148] vfs_write+0xc4/0x2e4
[ 303.305601] ksys_write+0x70/0x104
[ 303.306073] __arm64_sys_write+0x1c/0x28
[ 303.306622] invoke_syscall+0x48/0x114
[ 303.307156] el0_svc_common.constprop.0+0x44/0xec
[ 303.307803] do_el0_svc+0x38/0x98
[ 303.308268] el0_svc+0x2c/0x84
[ 303.308702] el0t_64_sync_handler+0xf4/0x120
[ 303.309293] el0t_64_sync+0x190/0x194
[ 303.309794] Code: ???????? ???????? ???????? ???????? (????????)
[ 303.310612] ---[ end trace 0000000000000000 ]---
Fix this by adding a .reconfigure() function to the fs operations which
we can use to check the requested flags and deny anything that's not RO
if the firmware doesn't implement SetVariable at runtime.
Fixes: f88814cc2578 ("efi/efivars: Expose RT service availability via efivars abstraction")
Signed-off-by: Ilias Apalodimas <[email protected]>
---
fs/efivarfs/super.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 0f6e4d223aea..942e748a4e03 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -15,6 +15,7 @@
#include <linux/magic.h>
#include <linux/statfs.h>
#include <linux/notifier.h>
+#include <linux/printk.h>
#include "internal.h"
@@ -300,8 +301,19 @@ static int efivarfs_get_tree(struct fs_context *fc)
return get_tree_single(fc, efivarfs_fill_super);
}
+static int efivarfs_reconfigure(struct fs_context *fc)
+{
+ if (!efivar_supports_writes() && !(fc->sb_flags & SB_RDONLY)) {
+ pr_err("Firmware does not support SetVariableRT. Can not remount with rw\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct fs_context_operations efivarfs_context_ops = {
.get_tree = efivarfs_get_tree,
+ .reconfigure = efivarfs_reconfigure,
};
static int efivarfs_init_fs_context(struct fs_context *fc)
--
2.30.2
Hi Jan,
On Mon, 7 Aug 2023 at 05:53, Masahisa Kojima <[email protected]> wrote:
>
> This series introduces the tee based EFI Runtime Variable Service.
>
> The eMMC device is typically owned by the non-secure world(linux in
> this case). There is an existing solution utilizing eMMC RPMB partition
> for EFI Variables, it is implemented by interacting with
> OP-TEE, StandaloneMM(as EFI Variable Service Pseudo TA), eMMC driver
> and tee-supplicant. The last piece is the tee-based variable access
> driver to interact with OP-TEE and StandaloneMM.
>
> Changelog:
> v7 -> v8
> Only patch #3 "efi: Add tee-based EFI variable driver" is updated.
> - fix typos
> - refactor error handling, direct return if applicable
> - use devm_add_action_or_reset() for closing of tee context/session
> - remove obvious comment
Any chance you can run this and see if it solves your issues?
Thanks
/Ilias
>
> v6 -> v7
> Patch #1-#4 are not updated.
> Patch #5 is added into this series, original patch is here:
> https://lore.kernel.org/all/[email protected]/
>
> There are two issues in the v6 series and v7 series addresses those.
>
> 1) efivar ops is not restored when the tee-supplicant daemon terminates.
> -> As the following patch says, user must remove the device before
> terminating tee-supplicant daemon.
> https://lore.kernel.org/all/[email protected]/
>
> 2) cause panic when someone remounts the efivarfs as RW even if
> SetVariable is not supported
> -> The fifth patch addresses this issue.
> "[PATCH v7 5/5] efivarfs: force RO when remounting if SetVariable is
> not supported"
>
> v5 -> v6
> - new patch #4 is added in this series, #1-#3 patches are unchanged.
> automatically update super block flag when the efivarops support
> SetVariable runtime service, so that user does not need to manually
> remount the efivarfs as RW.
>
> v4 -> v5
> - rebase to efi-next based on v6.4-rc1
> - set generic_ops.query_variable_info, it works as expected as follows.
> $ df -h /sys/firmware/efi/efivars/
> Filesystem Size Used Avail Use% Mounted on
> efivarfs 16K 1.3K 15K 8% /sys/firmware/efi/efivars
>
> v3 -> v4:
> - replace the reference from EDK2 to PI Specification
> - remove EDK2 source code reference comments
> - prepare nonblocking variant of set_variable, it just returns
> EFI_UNSUPPORTED
> - remove redundant buffer size check
> - argument name change in mm_communicate
> - function interface changes in setup_mm_hdr to remove (void **) cast
>
> v2 -> v3:
> - add CONFIG_EFI dependency to TEE_STMM_EFI
> - add missing return code check for tee_client_invoke_func()
> - directly call efivars_register/unregister from tee_stmm_efi.c
>
> rfc v1 -> v2:
> - split patch into three patches, one for drivers/tee,
> one for include/linux/efi.h, and one for the driver/firmware/efi/stmm
> - context/session management into probe() and remove() same as other tee
> client driver
> - StMM variable driver is moved from driver/tee/optee to driver/firmware/efi
> - use "tee" prefix instead of "optee" in driver/firmware/efi/stmm/tee_stmm_efi.c,
> this file does not contain op-tee specific code, abstracted by tee layer and
> StMM variable driver will work on other tee implementation.
> - PTA_STMM_CMD_COMMUNICATE -> PTA_STMM_CMD_COMMUNICATE
> - implement query_variable_store() but currently not used
> - no use of TEEC_SUCCESS, it is defined in driver/tee/optee/optee_private.h.
> Other tee client drivers use 0 instead of using TEEC_SUCCESS
> - remove TEEC_ERROR_EXCESS_DATA status, it is referred just to output
> error message
>
>
>
> Ilias Apalodimas (1):
> efivarfs: force RO when remounting if SetVariable is not supported
>
> Masahisa Kojima (4):
> efi: expose efivar generic ops register function
> efi: Add EFI_ACCESS_DENIED status code
> efi: Add tee-based EFI variable driver
> efivarfs: automatically update super block flag
>
> drivers/firmware/efi/Kconfig | 15 +
> drivers/firmware/efi/Makefile | 1 +
> drivers/firmware/efi/efi.c | 18 +
> drivers/firmware/efi/stmm/mm_communication.h | 236 +++++++
> drivers/firmware/efi/stmm/tee_stmm_efi.c | 612 +++++++++++++++++++
> drivers/firmware/efi/vars.c | 8 +
> fs/efivarfs/super.c | 45 ++
> include/linux/efi.h | 12 +
> 8 files changed, 947 insertions(+)
> create mode 100644 drivers/firmware/efi/stmm/mm_communication.h
> create mode 100644 drivers/firmware/efi/stmm/tee_stmm_efi.c
>
>
> base-commit: f6e6e95ce16205025b7b8680a66c30a0c4ec2270
> --
> 2.30.2
>
On Thu, 17 Aug 2023 at 12:22, Sumit Garg <[email protected]> wrote:
>
> On Wed, 16 Aug 2023 at 19:37, Jan Kiszka <[email protected]> wrote:
> >
> > On 16.08.23 13:58, Ilias Apalodimas wrote:
> > > On Tue, 15 Aug 2023 at 05:41, Masahisa Kojima
> > > <[email protected]> wrote:
> > >>
> > >> Hi Jan,
> > >>
> > >> 2023年8月15日(火) 2:23 Jan Kiszka <[email protected]>:
> > >>>
> > >>> On 14.08.23 11:24, Ilias Apalodimas wrote:
> > >>>> Hi Jan,
> > >>>>
> > >>>> On Mon, 7 Aug 2023 at 05:53, Masahisa Kojima <[email protected]> wrote:
> > >>>>>
> > >>>>> This series introduces the tee based EFI Runtime Variable Service.
> > >>>>>
> > >>>>> The eMMC device is typically owned by the non-secure world(linux in
> > >>>>> this case). There is an existing solution utilizing eMMC RPMB partition
> > >>>>> for EFI Variables, it is implemented by interacting with
> > >>>>> OP-TEE, StandaloneMM(as EFI Variable Service Pseudo TA), eMMC driver
> > >>>>> and tee-supplicant. The last piece is the tee-based variable access
> > >>>>> driver to interact with OP-TEE and StandaloneMM.
> > >>>>>
> > >>>>> Changelog:
> > >>>>> v7 -> v8
> > >>>>> Only patch #3 "efi: Add tee-based EFI variable driver" is updated.
> > >>>>> - fix typos
> > >>>>> - refactor error handling, direct return if applicable
> > >>>>> - use devm_add_action_or_reset() for closing of tee context/session
> > >>>>> - remove obvious comment
> > >>>>
> > >>>> Any chance you can run this and see if it solves your issues?
> > >>>>
> > >>>
> > >>> I also need [1], and I still need a cleanup script before terminating
> > >>> the tee-supplicant, right?
> > >>
> > >>
> > >> Yes, we need patch[1] and a cleanup script.
> > >> Sorry, I should note in the cover letter.
> > >>
> > >>> And if need some service in the initrd
> > >>> already, I still need to start the supplicant there and transfer its
> > >>> ownership to systemd later on?
> > >>
> > >> Yes.
> > >>
> > >>> These patches here only make life easier
> > >>> if the supplicant is started by systemd, after efivarfs has been
> > >>> mounted, correct?
> > >
> > > Not systemd specifically. Any tool that can signal
> > > <dev>/driver/unbind would work. Sumit is just reusing the default
> > > unbind notification mechanism
> > >
> >
> > I was referring to the boot ordering topic, not the shutdown issue.
> >
> > The latter has now a nicer way to trigger the device shutdown prior to
> > killing tee-supplicant, but you still need to do that explicitly, no?
> >
>
> Yeah it has to be done explicitly in user-space. As you have already
> seen, my first try (v1 patch) to do it in kernel space failed. The
> reason being that when those devices are being removed, the
> tee-supplicant has to be alive to handle RPC calls. The kernel only
> gets notified once "/dev/teepriv0" fd is closed and by that time
> tee-supplicant is already dead.
Yea, that was along the lines of what I asked in a past mail. IOW
bind the cleanups needed on the opening/closing of the
"/dev/teepriv0", but unfortunately that's not doable. What we could
do as a future enhancement is add a signal handler in the supplicant
that signals the events without relying on a different userspace app
to do that.
Sumit pointed out a few things we need to be cautious about on the
signal handler, but in any case, that's orthogonal to the current
approach.
Thanks
/Ilias
>
> -Sumit
>
> > Jan
> >
> > --
> > Siemens AG, Technology
> > Linux Expert Center
> >