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: 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
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 | 638 +++++++++++++++++++ drivers/firmware/efi/vars.c | 8 + fs/efivarfs/super.c | 33 + include/linux/efi.h | 12 + 8 files changed, 961 insertions(+) create mode 100644 drivers/firmware/efi/stmm/mm_communication.h create mode 100644 drivers/firmware/efi/stmm/tee_stmm_efi.c
base-commit: d0a1865cf7e2211d9227592ef4141f4632e33908
This is a preparation for supporting efivar operations provided by other than efi subsystem. Both register and unregister functions are exposed so that non-efi subsystem can revert the efi generic operation.
Acked-by: Sumit Garg sumit.garg@linaro.org Co-developed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- drivers/firmware/efi/efi.c | 12 ++++++++++++ include/linux/efi.h | 3 +++ 2 files changed, 15 insertions(+)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index d0dfa007bffc..d108cf03e19d 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -228,6 +228,18 @@ static void generic_ops_unregister(void) efivars_unregister(&generic_efivars); }
+void efivars_generic_ops_register(void) +{ + generic_ops_register(); +} +EXPORT_SYMBOL_GPL(efivars_generic_ops_register); + +void efivars_generic_ops_unregister(void) +{ + generic_ops_unregister(); +} +EXPORT_SYMBOL_GPL(efivars_generic_ops_unregister); + #ifdef CONFIG_EFI_CUSTOM_SSDT_OVERLAYS #define EFIVAR_SSDT_NAME_MAX 16UL static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; diff --git a/include/linux/efi.h b/include/linux/efi.h index bed3c92cbc31..b8ba9c5adc7a 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1349,4 +1349,7 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table) return xen_efi_config_table_is_usable(guid, table); }
+void efivars_generic_ops_register(void); +void efivars_generic_ops_unregister(void); + #endif /* _LINUX_EFI_H */
This commit adds the EFI_ACCESS_DENIED status code.
Acked-by: Sumit Garg sumit.garg@linaro.org Co-developed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- include/linux/efi.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/linux/efi.h b/include/linux/efi.h index b8ba9c5adc7a..657f7e203374 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -39,6 +39,7 @@ #define EFI_WRITE_PROTECTED ( 8 | (1UL << (BITS_PER_LONG-1))) #define EFI_OUT_OF_RESOURCES ( 9 | (1UL << (BITS_PER_LONG-1))) #define EFI_NOT_FOUND (14 | (1UL << (BITS_PER_LONG-1))) +#define EFI_ACCESS_DENIED (15 | (1UL << (BITS_PER_LONG-1))) #define EFI_TIMEOUT (18 | (1UL << (BITS_PER_LONG-1))) #define EFI_ABORTED (21 | (1UL << (BITS_PER_LONG-1))) #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
When the flash is not owned by the non-secure world, accessing the EFI variables is straightforward and done via EFI Runtime Variable Services. In this case, critical variables for system integrity and security are normally stored in the dedicated secure storage and only accessible from the secure world.
On the other hand, the small embedded devices don't have the special dedicated secure storage. The eMMC device with an RPMB partition is becoming more common, we can use an RPMB partition to store the EFI Variables.
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 TEE(OP-TEE in this case), 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 TEE and StandaloneMM.
So let's add the kernel functions needed.
This feature is implemented as a kernel module. StMM PTA has TA_FLAG_DEVICE_ENUM_SUPP flag when registered to OP-TEE so that this tee_stmm_efi module is probed after tee-supplicant starts, since "SetVariable" EFI Runtime Variable Service requires to interact with tee-supplicant.
Acked-by: Sumit Garg sumit.garg@linaro.org Co-developed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- drivers/firmware/efi/Kconfig | 15 + drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/stmm/mm_communication.h | 236 +++++++ drivers/firmware/efi/stmm/tee_stmm_efi.c | 638 +++++++++++++++++++ 4 files changed, 890 insertions(+) create mode 100644 drivers/firmware/efi/stmm/mm_communication.h create mode 100644 drivers/firmware/efi/stmm/tee_stmm_efi.c
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 043ca31c114e..aa38089d1e4a 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -287,3 +287,18 @@ config UEFI_CPER_X86 bool depends on UEFI_CPER && X86 default y + +config TEE_STMM_EFI + tristate "TEE based EFI runtime variable service driver" + depends on EFI && OPTEE && !EFI_VARS_PSTORE + help + Select this config option if TEE is compiled to include StandAloneMM + as a separate secure partition it has the ability to check and store + EFI variables on an RPMB or any other non-volatile medium used by + StandAloneMM. + + Enabling this will change the EFI runtime services from the firmware + provided functions to TEE calls. + + To compile this driver as a module, choose M here: the module + will be called tee_stmm_efi. diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile index b51f2a4c821e..2ca8ee6ab490 100644 --- a/drivers/firmware/efi/Makefile +++ b/drivers/firmware/efi/Makefile @@ -41,3 +41,4 @@ obj-$(CONFIG_EFI_CAPSULE_LOADER) += capsule-loader.o obj-$(CONFIG_EFI_EARLYCON) += earlycon.o obj-$(CONFIG_UEFI_CPER_ARM) += cper-arm.o obj-$(CONFIG_UEFI_CPER_X86) += cper-x86.o +obj-$(CONFIG_TEE_STMM_EFI) += stmm/tee_stmm_efi.o diff --git a/drivers/firmware/efi/stmm/mm_communication.h b/drivers/firmware/efi/stmm/mm_communication.h new file mode 100644 index 000000000000..52a1f32cd1eb --- /dev/null +++ b/drivers/firmware/efi/stmm/mm_communication.h @@ -0,0 +1,236 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Headers for EFI variable service via StandAloneMM, EDK2 application running + * in OP-TEE. Most of the structs and defines resemble the EDK2 naming. + * + * Copyright (c) 2017, Intel Corporation. All rights reserved. + * Copyright (C) 2020 Linaro Ltd. + */ + +#ifndef _MM_COMMUNICATION_H_ +#define _MM_COMMUNICATION_H_ + +/* + * Interface to the pseudo Trusted Application (TA), which provides a + * communication channel with the Standalone MM (Management Mode) + * Secure Partition running at Secure-EL0 + */ + +#define PTA_STMM_CMD_COMMUNICATE 0 + +/* + * Defined in OP-TEE, this UUID is used to identify the pseudo-TA. + * OP-TEE is using big endian GUIDs while UEFI uses little endian ones + */ +#define PTA_STMM_UUID \ + UUID_INIT(0xed32d533, 0x99e6, 0x4209, \ + 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7) + +#define EFI_MM_VARIABLE_GUID \ + EFI_GUID(0xed32d533, 0x99e6, 0x4209, \ + 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7) + +/** + * struct efi_mm_communicate_header - Header used for SMM variable communication + + * @header_guid: header use for disambiguation of content + * @message_len: length of the message. Does not include the size of the + * header + * @data: payload of the message + * + * Defined in the PI spec as EFI_MM_COMMUNICATE_HEADER. + * To avoid confusion in interpreting frames, the communication buffer should + * always begin with efi_mm_communicate_header. + */ +struct efi_mm_communicate_header { + efi_guid_t header_guid; + size_t message_len; + u8 data[]; +} __packed; + +#define MM_COMMUNICATE_HEADER_SIZE \ + (sizeof(struct efi_mm_communicate_header)) + +/* SPM return error codes */ +#define ARM_SVC_SPM_RET_SUCCESS 0 +#define ARM_SVC_SPM_RET_NOT_SUPPORTED -1 +#define ARM_SVC_SPM_RET_INVALID_PARAMS -2 +#define ARM_SVC_SPM_RET_DENIED -3 +#define ARM_SVC_SPM_RET_NO_MEMORY -5 + +#define SMM_VARIABLE_FUNCTION_GET_VARIABLE 1 +/* + * The payload for this function is + * SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME. + */ +#define SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME 2 +/* + * The payload for this function is SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE. + */ +#define SMM_VARIABLE_FUNCTION_SET_VARIABLE 3 +/* + * The payload for this function is + * SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO. + */ +#define SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO 4 +/* + * It is a notify event, no extra payload for this function. + */ +#define SMM_VARIABLE_FUNCTION_READY_TO_BOOT 5 +/* + * It is a notify event, no extra payload for this function. + */ +#define SMM_VARIABLE_FUNCTION_EXIT_BOOT_SERVICE 6 +/* + * The payload for this function is VARIABLE_INFO_ENTRY. + * The GUID in EFI_SMM_COMMUNICATE_HEADER is gEfiSmmVariableProtocolGuid. + */ +#define SMM_VARIABLE_FUNCTION_GET_STATISTICS 7 +/* + * The payload for this function is SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE + */ +#define SMM_VARIABLE_FUNCTION_LOCK_VARIABLE 8 + +#define SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_SET 9 + +#define SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET 10 + +#define SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE 11 +/* + * The payload for this function is + * SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT + */ +#define SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT 12 + +#define SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE 13 +/* + * The payload for this function is + * SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO + */ +#define SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO 14 + +/** + * struct smm_variable_communicate_header - Used for SMM variable communication + + * @function: function to call in Smm. + * @ret_status: return status + * @data: payload + */ +struct smm_variable_communicate_header { + size_t function; + efi_status_t ret_status; + u8 data[]; +}; + +#define MM_VARIABLE_COMMUNICATE_SIZE \ + (sizeof(struct smm_variable_communicate_header)) + +/** + * struct smm_variable_access - Used to communicate with StMM by + * SetVariable and GetVariable. + + * @guid: vendor GUID + * @data_size: size of EFI variable data + * @name_size: size of EFI name + * @attr: attributes + * @name: variable name + * + */ +struct smm_variable_access { + efi_guid_t guid; + size_t data_size; + size_t name_size; + u32 attr; + u16 name[]; +}; + +#define MM_VARIABLE_ACCESS_HEADER_SIZE \ + (sizeof(struct smm_variable_access)) +/** + * struct smm_variable_payload_size - Used to get the max allowed + * payload used in StMM. + * + * @size: size to fill in + * + */ +struct smm_variable_payload_size { + size_t size; +}; + +/** + * struct smm_variable_getnext - Used to communicate with StMM for + * GetNextVariableName. + * + * @guid: vendor GUID + * @name_size: size of the name of the variable + * @name: variable name + * + */ +struct smm_variable_getnext { + efi_guid_t guid; + size_t name_size; + u16 name[]; +}; + +#define MM_VARIABLE_GET_NEXT_HEADER_SIZE \ + (sizeof(struct smm_variable_getnext)) + +/** + * struct smm_variable_query_info - Used to communicate with StMM for + * QueryVariableInfo. + * + * @max_variable_storage: max available storage + * @remaining_variable_storage: remaining available storage + * @max_variable_size: max variable supported size + * @attr: attributes to query storage for + * + */ +struct smm_variable_query_info { + u64 max_variable_storage; + u64 remaining_variable_storage; + u64 max_variable_size; + u32 attr; +}; + +#define VAR_CHECK_VARIABLE_PROPERTY_REVISION 0x0001 +#define VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY BIT(0) +/** + * struct var_check_property - Used to store variable properties in StMM + * + * @revision: magic revision number for variable property checking + * @property: properties mask for the variable used in StMM. + * Currently RO flag is supported + * @attributes: variable attributes used in StMM checking when properties + * for a variable are enabled + * @minsize: minimum allowed size for variable payload checked against + * smm_variable_access->datasize in StMM + * @maxsize: maximum allowed size for variable payload checked against + * smm_variable_access->datasize in StMM + * + */ +struct var_check_property { + u16 revision; + u16 property; + u32 attributes; + size_t minsize; + size_t maxsize; +}; + +/** + * struct smm_variable_var_check_property - Used to communicate variable + * properties with StMM + * + * @guid: vendor GUID + * @name_size: size of EFI name + * @property: variable properties struct + * @name: variable name + * + */ +struct smm_variable_var_check_property { + efi_guid_t guid; + size_t name_size; + struct var_check_property property; + u16 name[]; +}; + +#endif /* _MM_COMMUNICATION_H_ */ diff --git a/drivers/firmware/efi/stmm/tee_stmm_efi.c b/drivers/firmware/efi/stmm/tee_stmm_efi.c new file mode 100644 index 000000000000..f6623171ae04 --- /dev/null +++ b/drivers/firmware/efi/stmm/tee_stmm_efi.c @@ -0,0 +1,638 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * EFI variable service via TEE + * + * Copyright (C) 2022 Linaro + */ + +#include <linux/efi.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/tee.h> +#include <linux/tee_drv.h> +#include <linux/ucs2_string.h> +#include "mm_communication.h" + +static struct efivars tee_efivars; +static struct efivar_operations tee_efivar_ops; + +static size_t max_buffer_size; /* comm + var + func + data */ +static size_t max_payload_size; /* func + data */ + +struct tee_stmm_efi_private { + struct tee_context *ctx; + u32 session; + struct device *dev; +}; + +static struct tee_stmm_efi_private pvt_data; + +/* UUID of the stmm PTA */ +static const struct tee_client_device_id tee_stmm_efi_id_table[] = { + {PTA_STMM_UUID}, + {} +}; + +static int tee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) +{ + /* currently only OP-TEE is supported as a communication path */ + if (ver->impl_id == TEE_IMPL_ID_OPTEE) + return 1; + else + return 0; +} + +/** + * tee_mm_communicate() - Pass a buffer to StandaloneMM running in TEE + * + * @comm_buf: locally allocated communication buffer + * @dsize: buffer size + * Return: status code + */ +static efi_status_t tee_mm_communicate(void *comm_buf, size_t dsize) +{ + size_t buf_size; + efi_status_t ret; + struct efi_mm_communicate_header *mm_hdr; + struct tee_ioctl_invoke_arg arg; + struct tee_param param[4]; + struct tee_shm *shm = NULL; + int rc; + + if (!comm_buf) + return EFI_INVALID_PARAMETER; + + mm_hdr = (struct efi_mm_communicate_header *)comm_buf; + buf_size = mm_hdr->message_len + sizeof(efi_guid_t) + sizeof(size_t); + + if (dsize != buf_size) + return EFI_INVALID_PARAMETER; + + shm = tee_shm_register_kernel_buf(pvt_data.ctx, comm_buf, buf_size); + if (IS_ERR(shm)) { + dev_err(pvt_data.dev, "Unable to register shared memory\n"); + return EFI_UNSUPPORTED; + } + + memset(&arg, 0, sizeof(arg)); + arg.func = PTA_STMM_CMD_COMMUNICATE; + arg.session = pvt_data.session; + arg.num_params = 4; + + memset(param, 0, sizeof(param)); + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT; + param[0].u.memref.size = buf_size; + param[0].u.memref.shm = shm; + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT; + param[2].attr = TEE_IOCTL_PARAM_ATTR_TYPE_NONE; + param[3].attr = TEE_IOCTL_PARAM_ATTR_TYPE_NONE; + + rc = tee_client_invoke_func(pvt_data.ctx, &arg, param); + tee_shm_free(shm); + + if (rc < 0 || arg.ret != 0) { + dev_err(pvt_data.dev, + "PTA_STMM_CMD_COMMUNICATE invoke error: 0x%x\n", arg.ret); + return EFI_DEVICE_ERROR; + } + + switch (param[1].u.value.a) { + case ARM_SVC_SPM_RET_SUCCESS: + ret = EFI_SUCCESS; + break; + + case ARM_SVC_SPM_RET_INVALID_PARAMS: + ret = EFI_INVALID_PARAMETER; + break; + + case ARM_SVC_SPM_RET_DENIED: + ret = EFI_ACCESS_DENIED; + break; + + case ARM_SVC_SPM_RET_NO_MEMORY: + ret = EFI_OUT_OF_RESOURCES; + break; + + default: + ret = EFI_ACCESS_DENIED; + } + + return ret; +} + +/** + * mm_communicate() - Adjust the communication buffer to StandAlonneMM and send + * it to TEE + * + * @comm_buf: locally allocated communication buffer, buffer should + * be enough big to have some headers and payload + * @payload_size: payload size + * Return: status code + */ +static efi_status_t mm_communicate(u8 *comm_buf, size_t payload_size) +{ + size_t dsize; + efi_status_t ret; + struct efi_mm_communicate_header *mm_hdr; + struct smm_variable_communicate_header *var_hdr; + + dsize = payload_size + MM_COMMUNICATE_HEADER_SIZE + + MM_VARIABLE_COMMUNICATE_SIZE; + mm_hdr = (struct efi_mm_communicate_header *)comm_buf; + var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data; + + ret = tee_mm_communicate(comm_buf, dsize); + if (ret != EFI_SUCCESS) { + dev_err(pvt_data.dev, "%s failed!\n", __func__); + return ret; + } + + return var_hdr->ret_status; +} + +/** + * setup_mm_hdr() - Allocate a buffer for StandAloneMM and initialize the + * header data. + * + * @dptr: pointer address to store allocated buffer + * @payload_size: payload size + * @func: standAloneMM function number + * @ret: EFI return code + * Return: pointer to corresponding StandAloneMM function buffer or NULL + */ +static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func, + efi_status_t *ret) +{ + const efi_guid_t mm_var_guid = EFI_MM_VARIABLE_GUID; + struct efi_mm_communicate_header *mm_hdr; + struct smm_variable_communicate_header *var_hdr; + u8 *comm_buf; + + /* In the init function we initialize max_buffer_size with + * get_max_payload(). So skip the test if max_buffer_size is initialized + * StandAloneMM will perform similar checks and drop the buffer if it's + * too long + */ + if (max_buffer_size && + max_buffer_size < (MM_COMMUNICATE_HEADER_SIZE + + MM_VARIABLE_COMMUNICATE_SIZE + payload_size)) { + *ret = EFI_INVALID_PARAMETER; + return NULL; + } + + comm_buf = kzalloc(MM_COMMUNICATE_HEADER_SIZE + + MM_VARIABLE_COMMUNICATE_SIZE + payload_size, + GFP_KERNEL); + if (!comm_buf) { + *ret = EFI_OUT_OF_RESOURCES; + return NULL; + } + + mm_hdr = (struct efi_mm_communicate_header *)comm_buf; + memcpy(&mm_hdr->header_guid, &mm_var_guid, sizeof(mm_hdr->header_guid)); + mm_hdr->message_len = MM_VARIABLE_COMMUNICATE_SIZE + payload_size; + + var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data; + var_hdr->function = func; + if (dptr) + *dptr = comm_buf; + *ret = EFI_SUCCESS; + + return var_hdr->data; +} + +/** + * get_max_payload() - Get variable payload size from StandAloneMM. + * + * @size: size of the variable in storage + * Return: status code + */ +static efi_status_t get_max_payload(size_t *size) +{ + struct smm_variable_payload_size *var_payload = NULL; + size_t payload_size; + u8 *comm_buf = NULL; + efi_status_t ret; + + if (!size) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + payload_size = sizeof(*var_payload); + var_payload = setup_mm_hdr(&comm_buf, payload_size, + SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE, + &ret); + if (!comm_buf) + goto out; + + ret = mm_communicate(comm_buf, payload_size); + if (ret != EFI_SUCCESS) + goto out; + + /* Make sure the buffer is big enough for storing variables */ + if (var_payload->size < MM_VARIABLE_ACCESS_HEADER_SIZE + 0x20) { + ret = EFI_DEVICE_ERROR; + goto out; + } + *size = var_payload->size; + /* + * There seems to be a bug in EDK2 miscalculating the boundaries and + * size checks, so deduct 2 more bytes to fulfill this requirement. Fix + * it up here to ensure backwards compatibility with older versions + * (cf. StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c. + * sizeof (EFI_MM_COMMUNICATE_HEADER) instead the size minus the + * flexible array member). + * + * size is guaranteed to be > 2 due to checks on the beginning. + */ + *size -= 2; +out: + kfree(comm_buf); + return ret; +} + +static efi_status_t get_property_int(u16 *name, size_t name_size, + const efi_guid_t *vendor, + struct var_check_property *var_property) +{ + struct smm_variable_var_check_property *smm_property; + size_t payload_size; + u8 *comm_buf = NULL; + efi_status_t ret; + + memset(var_property, 0, sizeof(*var_property)); + payload_size = sizeof(*smm_property) + name_size; + if (payload_size > max_payload_size) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + smm_property = setup_mm_hdr( + &comm_buf, payload_size, + SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET, &ret); + if (!comm_buf) + goto out; + + memcpy(&smm_property->guid, vendor, sizeof(smm_property->guid)); + smm_property->name_size = name_size; + memcpy(smm_property->name, name, name_size); + + ret = mm_communicate(comm_buf, payload_size); + /* + * Currently only R/O property is supported in StMM. + * Variables that are not set to R/O will not set the property in StMM + * and the call will return EFI_NOT_FOUND. We are setting the + * properties to 0x0 so checking against that is enough for the + * EFI_NOT_FOUND case. + */ + if (ret == EFI_NOT_FOUND) + ret = EFI_SUCCESS; + if (ret != EFI_SUCCESS) + goto out; + memcpy(var_property, &smm_property->property, sizeof(*var_property)); + +out: + kfree(comm_buf); + return ret; +} + +static efi_status_t tee_get_variable(u16 *name, efi_guid_t *vendor, + u32 *attributes, unsigned long *data_size, + void *data) +{ + struct var_check_property var_property; + struct smm_variable_access *var_acc; + size_t payload_size; + size_t name_size; + size_t tmp_dsize; + u8 *comm_buf = NULL; + efi_status_t ret; + + if (!name || !vendor || !data_size) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + name_size = (ucs2_strnlen(name, EFI_VAR_NAME_LEN) + 1) * sizeof(u16); + if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + /* Trim output buffer size */ + tmp_dsize = *data_size; + if (name_size + tmp_dsize > + max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) { + tmp_dsize = max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE - + name_size; + } + + /* Get communication buffer and initialize header */ + payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + tmp_dsize; + var_acc = setup_mm_hdr(&comm_buf, payload_size, + SMM_VARIABLE_FUNCTION_GET_VARIABLE, &ret); + if (!comm_buf) + goto out; + + /* Fill in contents */ + memcpy(&var_acc->guid, vendor, sizeof(var_acc->guid)); + var_acc->data_size = tmp_dsize; + var_acc->name_size = name_size; + var_acc->attr = attributes ? *attributes : 0; + memcpy(var_acc->name, name, name_size); + + /* Communicate */ + ret = mm_communicate(comm_buf, payload_size); + if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) + /* Update with reported data size for trimmed case */ + *data_size = var_acc->data_size; + if (ret != EFI_SUCCESS) + goto out; + + ret = get_property_int(name, name_size, vendor, &var_property); + if (ret != EFI_SUCCESS) + goto out; + + if (attributes) + *attributes = var_acc->attr; + + if (data) + memcpy(data, (u8 *)var_acc->name + var_acc->name_size, + var_acc->data_size); + else + ret = EFI_INVALID_PARAMETER; + +out: + kfree(comm_buf); + return ret; +} + +static efi_status_t tee_get_next_variable(unsigned long *name_size, + efi_char16_t *name, efi_guid_t *guid) +{ + struct smm_variable_getnext *var_getnext; + size_t payload_size; + size_t out_name_size; + size_t in_name_size; + u8 *comm_buf = NULL; + efi_status_t ret; + + if (!name_size || !name || !guid) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + out_name_size = *name_size; + in_name_size = (ucs2_strnlen(name, EFI_VAR_NAME_LEN) + 1) * sizeof(u16); + + if (out_name_size < in_name_size) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + if (in_name_size > + max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + /* Trim output buffer size */ + if (out_name_size > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) + out_name_size = + max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE; + + payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE + out_name_size; + var_getnext = setup_mm_hdr(&comm_buf, payload_size, + SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME, + &ret); + if (!comm_buf) + goto out; + + /* Fill in contents */ + memcpy(&var_getnext->guid, guid, sizeof(var_getnext->guid)); + var_getnext->name_size = out_name_size; + memcpy(var_getnext->name, name, in_name_size); + memset((u8 *)var_getnext->name + in_name_size, 0x0, + out_name_size - in_name_size); + + /* Communicate */ + ret = mm_communicate(comm_buf, payload_size); + if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) { + /* Update with reported data size for trimmed case */ + *name_size = var_getnext->name_size; + } + if (ret != EFI_SUCCESS) + goto out; + + memcpy(guid, &var_getnext->guid, sizeof(*guid)); + memcpy(name, var_getnext->name, var_getnext->name_size); + +out: + kfree(comm_buf); + return ret; +} + +static efi_status_t tee_set_variable(efi_char16_t *name, efi_guid_t *vendor, + u32 attributes, unsigned long data_size, + void *data) +{ + efi_status_t ret; + struct var_check_property var_property; + struct smm_variable_access *var_acc; + size_t payload_size; + size_t name_size; + u8 *comm_buf = NULL; + + if (!name || name[0] == 0 || !vendor) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + if (data_size > 0 && !data) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + /* Check payload size */ + name_size = (ucs2_strnlen(name, EFI_VAR_NAME_LEN) + 1) * sizeof(u16); + payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size; + if (payload_size > max_payload_size) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + /* + * Allocate the buffer early, before switching to RW (if needed) + * so we won't need to account for any failures in reading/setting + * the properties, if the allocation fails + */ + var_acc = setup_mm_hdr(&comm_buf, payload_size, + SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret); + if (!comm_buf) + goto out; + + /* + * The API has the ability to override RO flags. If no RO check was + * requested switch the variable to RW for the duration of this call + */ + ret = get_property_int(name, name_size, vendor, &var_property); + if (ret != EFI_SUCCESS) { + dev_err(pvt_data.dev, "Getting variable property failed\n"); + goto out; + } + + if (var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY) { + ret = EFI_WRITE_PROTECTED; + goto out; + } + + /* Fill in contents */ + memcpy(&var_acc->guid, vendor, sizeof(var_acc->guid)); + var_acc->data_size = data_size; + var_acc->name_size = name_size; + var_acc->attr = attributes; + memcpy(var_acc->name, name, name_size); + memcpy((u8 *)var_acc->name + name_size, data, data_size); + + + /* Communicate */ + ret = mm_communicate(comm_buf, payload_size); + dev_dbg(pvt_data.dev, "Set Variable %s %d %lx\n", __FILE__, __LINE__, ret); +out: + kfree(comm_buf); + return ret; +} + +static efi_status_t tee_set_variable_nonblocking(efi_char16_t *name, + efi_guid_t *vendor, + u32 attributes, + unsigned long data_size, + void *data) +{ + return EFI_UNSUPPORTED; +} + +static efi_status_t tee_query_variable_info(u32 attributes, + u64 *max_variable_storage_size, + u64 *remain_variable_storage_size, + u64 *max_variable_size) +{ + struct smm_variable_query_info *mm_query_info; + size_t payload_size; + efi_status_t ret; + u8 *comm_buf; + + payload_size = sizeof(*mm_query_info); + mm_query_info = setup_mm_hdr(&comm_buf, payload_size, + SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO, + &ret); + if (!comm_buf) + goto out; + + mm_query_info->attr = attributes; + ret = mm_communicate(comm_buf, payload_size); + if (ret != EFI_SUCCESS) + goto out; + *max_variable_storage_size = mm_query_info->max_variable_storage; + *remain_variable_storage_size = + mm_query_info->remaining_variable_storage; + *max_variable_size = mm_query_info->max_variable_size; + +out: + kfree(comm_buf); + return ret; +} + +static int tee_stmm_efi_probe(struct device *dev) +{ + struct tee_ioctl_open_session_arg sess_arg; + efi_status_t ret; + int rc; + + /* Open context with TEE driver */ + pvt_data.ctx = tee_client_open_context(NULL, tee_ctx_match, NULL, NULL); + if (IS_ERR(pvt_data.ctx)) + return -ENODEV; + + /* Open session with StMM PTA */ + memset(&sess_arg, 0, sizeof(sess_arg)); + export_uuid(sess_arg.uuid, &tee_stmm_efi_id_table[0].uuid); + rc = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL); + if ((rc < 0) || (sess_arg.ret != 0)) { + dev_err(dev, "tee_client_open_session failed, err: %x\n", + sess_arg.ret); + rc = -EINVAL; + goto out_ctx; + } + pvt_data.session = sess_arg.session; + pvt_data.dev = dev; + + ret = get_max_payload(&max_payload_size); + if (ret != EFI_SUCCESS) { + rc = -EIO; + goto out_sess; + } + + max_buffer_size = MM_COMMUNICATE_HEADER_SIZE + + MM_VARIABLE_COMMUNICATE_SIZE + + max_payload_size; + + tee_efivar_ops.get_variable = tee_get_variable; + tee_efivar_ops.get_next_variable = tee_get_next_variable; + tee_efivar_ops.set_variable = tee_set_variable; + tee_efivar_ops.set_variable_nonblocking = tee_set_variable_nonblocking; + tee_efivar_ops.query_variable_store = efi_query_variable_store; + tee_efivar_ops.query_variable_info = tee_query_variable_info; + + efivars_generic_ops_unregister(); + pr_info("Use tee-based EFI runtime variable services\n"); + efivars_register(&tee_efivars, &tee_efivar_ops); + + return 0; + +out_sess: + tee_client_close_session(pvt_data.ctx, pvt_data.session); +out_ctx: + tee_client_close_context(pvt_data.ctx); + + return rc; +} + +static int tee_stmm_efi_remove(struct device *dev) +{ + efivars_unregister(&tee_efivars); + efivars_generic_ops_register(); + + tee_client_close_session(pvt_data.ctx, pvt_data.session); + tee_client_close_context(pvt_data.ctx); + + return 0; +} + +MODULE_DEVICE_TABLE(tee, tee_stmm_efi_id_table); + +static struct tee_client_driver tee_stmm_efi_driver = { + .id_table = tee_stmm_efi_id_table, + .driver = { + .name = "tee-stmm-efi", + .bus = &tee_bus_type, + .probe = tee_stmm_efi_probe, + .remove = tee_stmm_efi_remove, + }, +}; + +static int __init tee_stmm_efi_mod_init(void) +{ + return driver_register(&tee_stmm_efi_driver.driver); +} + +static void __exit tee_stmm_efi_mod_exit(void) +{ + driver_unregister(&tee_stmm_efi_driver.driver); +} + +module_init(tee_stmm_efi_mod_init); +module_exit(tee_stmm_efi_mod_exit); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Ilias Apalodimas ilias.apalodimas@linaro.org"); +MODULE_AUTHOR("Masahisa Kojima masahisa.kojima@linaro.org"); +MODULE_DESCRIPTION("TEE based EFI runtime variable service driver");
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- drivers/firmware/efi/efi.c | 6 ++++++ drivers/firmware/efi/vars.c | 8 ++++++++ fs/efivarfs/super.c | 33 +++++++++++++++++++++++++++++++++ include/linux/efi.h | 8 ++++++++ 4 files changed, 55 insertions(+)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index d108cf03e19d..00494fcf16ba 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -32,6 +32,7 @@ #include <linux/ucs2_string.h> #include <linux/memblock.h> #include <linux/security.h> +#include <linux/notifier.h>
#include <asm/early_ioremap.h>
@@ -184,6 +185,9 @@ static const struct attribute_group efi_subsys_attr_group = { .is_visible = efi_attr_is_visible, };
+struct blocking_notifier_head efivar_ops_nh; +EXPORT_SYMBOL_GPL(efivar_ops_nh); + static struct efivars generic_efivars; static struct efivar_operations generic_ops;
@@ -442,6 +446,8 @@ static int __init efisubsys_init(void) platform_device_register_simple("efivars", 0, NULL, 0); }
+ BLOCKING_INIT_NOTIFIER_HEAD(&efivar_ops_nh); + error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group); if (error) { pr_err("efi: Sysfs attribute export failed with error %d.\n", diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index e9dc7116daf1..f654e6f6af87 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c @@ -63,6 +63,7 @@ int efivars_register(struct efivars *efivars, const struct efivar_operations *ops) { int rv; + int event;
if (down_interruptible(&efivars_lock)) return -EINTR; @@ -77,6 +78,13 @@ int efivars_register(struct efivars *efivars,
__efivars = efivars;
+ if (efivar_supports_writes()) + event = EFIVAR_OPS_RDWR; + else + event = EFIVAR_OPS_RDONLY; + + blocking_notifier_call_chain(&efivar_ops_nh, event, NULL); + pr_info("Registered efivars operations\n"); rv = 0; out: diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c index e028fafa04f3..0f6e4d223aea 100644 --- a/fs/efivarfs/super.c +++ b/fs/efivarfs/super.c @@ -14,11 +14,36 @@ #include <linux/slab.h> #include <linux/magic.h> #include <linux/statfs.h> +#include <linux/notifier.h>
#include "internal.h"
LIST_HEAD(efivarfs_list);
+struct efivarfs_info { + struct super_block *sb; + struct notifier_block nb; +}; + +static struct efivarfs_info info; + +static int efivarfs_ops_notifier(struct notifier_block *nb, unsigned long event, + void *data) +{ + switch (event) { + case EFIVAR_OPS_RDONLY: + info.sb->s_flags |= SB_RDONLY; + break; + case EFIVAR_OPS_RDWR: + info.sb->s_flags &= ~SB_RDONLY; + break; + default: + return NOTIFY_DONE; + } + + return NOTIFY_OK; +} + static void efivarfs_evict_inode(struct inode *inode) { clear_inode(inode); @@ -255,6 +280,12 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc) if (!root) return -ENOMEM;
+ info.sb = sb; + info.nb.notifier_call = efivarfs_ops_notifier; + err = blocking_notifier_chain_register(&efivar_ops_nh, &info.nb); + if (err) + return err; + INIT_LIST_HEAD(&efivarfs_list);
err = efivar_init(efivarfs_callback, (void *)sb, true, &efivarfs_list); @@ -281,6 +312,8 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
static void efivarfs_kill_sb(struct super_block *sb) { + blocking_notifier_chain_unregister(&efivar_ops_nh, &info.nb); + info.sb = NULL; kill_litter_super(sb);
if (!efivar_is_available()) diff --git a/include/linux/efi.h b/include/linux/efi.h index 657f7e203374..2533e4f2547f 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1350,6 +1350,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table) return xen_efi_config_table_is_usable(guid, table); }
+/* + * efivar ops event type + */ +#define EFIVAR_OPS_RDONLY 0 +#define EFIVAR_OPS_RDWR 1 + +extern struct blocking_notifier_head efivar_ops_nh; + void efivars_generic_ops_register(void); void efivars_generic_ops_unregister(void);
On 22.06.23 10:51, Masahisa Kojima wrote:
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that will only cause panics there) when the daemon terminates, does it?
Jan
Hi Kojima-san, Jan
On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
On 22.06.23 10:51, Masahisa Kojima wrote:
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that will only cause panics there) when the daemon terminates, does it?
It doesn't, but I think I got a better way out. Even what you suggest won't solve the problem entirely. For the sake of context - The kernel decides between the RO/RW depending on the SetVariable ptr - The stmm *module* registers and swaps the RT calls -- and the ptr is now valid. Note here that the module probe function will run only if the supplicant is running - Once the module is inserted the filesystem will be remounted even without the supplicant running, which would not trigger an oops, but an hard to decipher error message from OP-TEE.
So even if we switch the permissions back to RO when the supplicant dies, someone can still remount it as RW and trigger the same error.
Which got me thinking and staring the TEE subsystem a bit more. The supplicant is backed by a /dev file, which naturally has .open() and .release() callbacks. Why don't we leave the module perform the initial setup -- e.g talk to StMM and make sure it's there, setup the necessary buffers etc and defer the actual swapping of the efivar ops and the filesystem permissions there? I might 'feel' a bit weird, but as I mentioned the module probe function only runs if the supplicant is running anyway
Cheers /Ilias
Jan
-- Siemens AG, Technology Competence Center Embedded Linux
Hi Ilias, Jan,
On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san, Jan
On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
On 22.06.23 10:51, Masahisa Kojima wrote:
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that will only cause panics there) when the daemon terminates, does it?
It doesn't, but I think I got a better way out. Even what you suggest won't solve the problem entirely. For the sake of context
- The kernel decides between the RO/RW depending on the SetVariable ptr
- The stmm *module* registers and swaps the RT calls -- and the ptr is now
valid. Note here that the module probe function will run only if the supplicant is running
- Once the module is inserted the filesystem will be remounted even without
the supplicant running, which would not trigger an oops, but an hard to decipher error message from OP-TEE.
So even if we switch the permissions back to RO when the supplicant dies, someone can still remount it as RW and trigger the same error.
Which got me thinking and staring the TEE subsystem a bit more. The supplicant is backed by a /dev file, which naturally has .open() and .release() callbacks. Why don't we leave the module perform the initial setup -- e.g talk to StMM and make sure it's there, setup the necessary buffers etc and defer the actual swapping of the efivar ops and the filesystem permissions there? I might 'feel' a bit weird, but as I mentioned the module probe function only runs if the supplicant is running anyway
I think we are discussing two issues.
1) efivar ops is not restored when the tee-supplicant daemon terminates.
The patch[1] sent by Sumit addresses this issue. Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called when the tee-supplicant daemon terminates, then restore the previous efivar ops and SB_RDONLY flag if necessary.
2) cause panic when someone remounts the efivarfs as RW even if SetVariable is not supported.
[1] https://lore.kernel.org/all/20230607151435.92654-1-sumit.garg@linaro.org/
Thanks, Masahisa Kojima
Cheers /Ilias
Jan
-- Siemens AG, Technology Competence Center Embedded Linux
Hi Kojima-san,
On Mon, 24 Jul 2023 at 05:53, Masahisa Kojima masahisa.kojima@linaro.org wrote:
Hi Ilias, Jan,
On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san, Jan
On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
On 22.06.23 10:51, Masahisa Kojima wrote:
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that will only cause panics there) when the daemon terminates, does it?
It doesn't, but I think I got a better way out. Even what you suggest won't solve the problem entirely. For the sake of context
- The kernel decides between the RO/RW depending on the SetVariable ptr
- The stmm *module* registers and swaps the RT calls -- and the ptr is now
valid. Note here that the module probe function will run only if the supplicant is running
- Once the module is inserted the filesystem will be remounted even without
the supplicant running, which would not trigger an oops, but an hard to decipher error message from OP-TEE.
So even if we switch the permissions back to RO when the supplicant dies, someone can still remount it as RW and trigger the same error.
Which got me thinking and staring the TEE subsystem a bit more. The supplicant is backed by a /dev file, which naturally has .open() and .release() callbacks. Why don't we leave the module perform the initial setup -- e.g talk to StMM and make sure it's there, setup the necessary buffers etc and defer the actual swapping of the efivar ops and the filesystem permissions there? I might 'feel' a bit weird, but as I mentioned the module probe function only runs if the supplicant is running anyway
I think we are discussing two issues.
Yes
- efivar ops is not restored when the tee-supplicant daemon terminates.
The patch[1] sent by Sumit addresses this issue. Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called when the tee-supplicant daemon terminates, then restore the previous efivar ops and SB_RDONLY flag if necessary.
Ok but that didn't fix the original error Jan reported and I am not sure about the patch status
- cause panic when someone remounts the efivarfs as RW even if
SetVariable is not supported.
Yes, this [0] is fixing that issue
[0] https://lore.kernel.org/linux-efi/20230609094532.562934-1-ilias.apalodimas@l... Thanks /Ilias
[1] https://lore.kernel.org/all/20230607151435.92654-1-sumit.garg@linaro.org/
Thanks, Masahisa Kojima
Cheers /Ilias
Jan
-- Siemens AG, Technology Competence Center Embedded Linux
Hi Ilias,
On Mon, 24 Jul 2023 at 19:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san,
On Mon, 24 Jul 2023 at 05:53, Masahisa Kojima masahisa.kojima@linaro.org wrote:
Hi Ilias, Jan,
On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san, Jan
On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
On 22.06.23 10:51, Masahisa Kojima wrote:
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that will only cause panics there) when the daemon terminates, does it?
It doesn't, but I think I got a better way out. Even what you suggest won't solve the problem entirely. For the sake of context
- The kernel decides between the RO/RW depending on the SetVariable ptr
- The stmm *module* registers and swaps the RT calls -- and the ptr is now
valid. Note here that the module probe function will run only if the supplicant is running
- Once the module is inserted the filesystem will be remounted even without
the supplicant running, which would not trigger an oops, but an hard to decipher error message from OP-TEE.
So even if we switch the permissions back to RO when the supplicant dies, someone can still remount it as RW and trigger the same error.
Which got me thinking and staring the TEE subsystem a bit more. The supplicant is backed by a /dev file, which naturally has .open() and .release() callbacks. Why don't we leave the module perform the initial setup -- e.g talk to StMM and make sure it's there, setup the necessary buffers etc and defer the actual swapping of the efivar ops and the filesystem permissions there? I might 'feel' a bit weird, but as I mentioned the module probe function only runs if the supplicant is running anyway
I think we are discussing two issues.
Yes
- efivar ops is not restored when the tee-supplicant daemon terminates.
The patch[1] sent by Sumit addresses this issue. Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called when the tee-supplicant daemon terminates, then restore the previous efivar ops and SB_RDONLY flag if necessary.
Ok but that didn't fix the original error Jan reported and I am not sure about the patch status
I think the patch is pending because the fTPM still causes panic when the system shuts down. https://lore.kernel.org/all/452472c5-ef30-ac30-6e4e-954f53b48315@siemens.com...
This is fTPM specific issue and is unrelated to the tee-based SetVariable runtime series itself.
- cause panic when someone remounts the efivarfs as RW even if
SetVariable is not supported.
Yes, this [0] is fixing that issue
Thank you, I will include this patch in the next submission.
Anyway, the GetVariable() runtime service backed by the U-Boot variable service does not work from kernel v6.4.0, so I will investigate this issue.
Thanks, Masahisa Kojima
[0] https://lore.kernel.org/linux-efi/20230609094532.562934-1-ilias.apalodimas@l... Thanks /Ilias
[1] https://lore.kernel.org/all/20230607151435.92654-1-sumit.garg@linaro.org/
Thanks, Masahisa Kojima
Cheers /Ilias
Jan
-- Siemens AG, Technology Competence Center Embedded Linux
On Wed, 26 Jul 2023 at 13:49, Masahisa Kojima masahisa.kojima@linaro.org wrote:
Hi Ilias,
On Mon, 24 Jul 2023 at 19:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san,
On Mon, 24 Jul 2023 at 05:53, Masahisa Kojima masahisa.kojima@linaro.org wrote:
Hi Ilias, Jan,
On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san, Jan
On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
On 22.06.23 10:51, Masahisa Kojima wrote:
efivar operation is updated when the tee_stmm_efi module is probed. tee_stmm_efi module supports SetVariable runtime service, but user needs to manually remount the efivarfs as RW to enable the write access if the previous efivar operation does not support SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to efivarfs subsystem, then drops SB_RDONLY flag if the efivar operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that will only cause panics there) when the daemon terminates, does it?
It doesn't, but I think I got a better way out. Even what you suggest won't solve the problem entirely. For the sake of context
- The kernel decides between the RO/RW depending on the SetVariable ptr
- The stmm *module* registers and swaps the RT calls -- and the ptr is now
valid. Note here that the module probe function will run only if the supplicant is running
- Once the module is inserted the filesystem will be remounted even without
the supplicant running, which would not trigger an oops, but an hard to decipher error message from OP-TEE.
So even if we switch the permissions back to RO when the supplicant dies, someone can still remount it as RW and trigger the same error.
Which got me thinking and staring the TEE subsystem a bit more. The supplicant is backed by a /dev file, which naturally has .open() and .release() callbacks. Why don't we leave the module perform the initial setup -- e.g talk to StMM and make sure it's there, setup the necessary buffers etc and defer the actual swapping of the efivar ops and the filesystem permissions there? I might 'feel' a bit weird, but as I mentioned the module probe function only runs if the supplicant is running anyway
I think we are discussing two issues.
Yes
- efivar ops is not restored when the tee-supplicant daemon terminates.
The patch[1] sent by Sumit addresses this issue. Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called when the tee-supplicant daemon terminates, then restore the previous efivar ops and SB_RDONLY flag if necessary.
Ok but that didn't fix the original error Jan reported and I am not sure about the patch status
I think the patch is pending because the fTPM still causes panic when the system shuts down. https://lore.kernel.org/all/452472c5-ef30-ac30-6e4e-954f53b48315@siemens.com...
This is fTPM specific issue and is unrelated to the tee-based SetVariable runtime series itself.
- cause panic when someone remounts the efivarfs as RW even if
SetVariable is not supported.
Yes, this [0] is fixing that issue
Thank you, I will include this patch in the next submission.
Anyway, the GetVariable() runtime service backed by the U-Boot variable service does not work from kernel v6.4.0, so I will investigate this issue.
I found that the QueryVariableInfo EFI API is required since this patch[2]. Current U-Boot does not support QueryVariableInfo runtime service. Anyway this is not directly related to this series. After efivar ops is replaced by the tee-based one, variable access works fine.
[2] https://lore.kernel.org/all/20230517153812.2010174-1-anisse@astier.eu/
Thanks, Masahisa Kojima
Thanks, Masahisa Kojima
[0] https://lore.kernel.org/linux-efi/20230609094532.562934-1-ilias.apalodimas@l... Thanks /Ilias
[1] https://lore.kernel.org/all/20230607151435.92654-1-sumit.garg@linaro.org/
Thanks, Masahisa Kojima
Cheers /Ilias
Jan
-- Siemens AG, Technology Competence Center Embedded Linux
On 22.06.23 10:51, Masahisa Kojima 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: 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.
But that is not yet resolving the architectural problem with that userspace daemon dependency. What are the next steps for that now?
Thanks, Jan
Hi Jan,
On Thu, 22 Jun 2023 at 17:56, Jan Kiszka jan.kiszka@siemens.com wrote:
On 22.06.23 10:51, Masahisa Kojima 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: 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.
But that is not yet resolving the architectural problem with that userspace daemon dependency. What are the next steps for that now?
We are trying to find some cycles to work on that, however, I don't have a time estimate on that. But the question is different here. Since this addresses the problems distros have wrt to SetVariableRT (even for a limited set of platforms) are we ok pulling this in? I can't think of a technical reason we shouldn't. The supplicant limitations are known and the firrmwareTPM has a similar set of problems.
Thanks /Ilias
Thanks, Jan
-- Siemens AG, Technology Competence Center Embedded Linux
On 22.06.23 17:04, Ilias Apalodimas wrote:
Hi Jan,
On Thu, 22 Jun 2023 at 17:56, Jan Kiszka jan.kiszka@siemens.com wrote:
On 22.06.23 10:51, Masahisa Kojima 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: 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.
But that is not yet resolving the architectural problem with that userspace daemon dependency. What are the next steps for that now?
We are trying to find some cycles to work on that, however, I don't have a time estimate on that. But the question is different here. Since this addresses the problems distros have wrt to SetVariableRT (even for a limited set of platforms) are we ok pulling this in? I can't think of a technical reason we shouldn't. The supplicant limitations are known and the firrmwareTPM has a similar set of problems.
It will not change we have to do on the distro side because we have to deal not only with the startup issue and StMM but also with fTPM and with shutdown. Only an in-kernel supplicant for RPMB would resolve that according to my understanding.
But the question is fair if we can evolve from this stage here to an in-kernel approach without causing breakages or other headache to distros adopting it (too early). That's why I asked for the roadmap.
Jan
On Thu, Jun 22, 2023 at 08:32:44PM +0200, Jan Kiszka wrote:
On 22.06.23 17:04, Ilias Apalodimas wrote:
Hi Jan,
On Thu, 22 Jun 2023 at 17:56, Jan Kiszka jan.kiszka@siemens.com wrote:
On 22.06.23 10:51, Masahisa Kojima 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: 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.
But that is not yet resolving the architectural problem with that userspace daemon dependency. What are the next steps for that now?
We are trying to find some cycles to work on that, however, I don't have a time estimate on that. But the question is different here. Since this addresses the problems distros have wrt to SetVariableRT (even for a limited set of platforms) are we ok pulling this in? I can't think of a technical reason we shouldn't. The supplicant limitations are known and the firrmwareTPM has a similar set of problems.
It will not change we have to do on the distro side because we have to deal not only with the startup issue and StMM but also with fTPM and with shutdown. Only an in-kernel supplicant for RPMB would resolve that according to my understanding.
Exactly and it's worth noting that even that will come with some minor limitations. E.g the randomseed variables set by the efistub currently won't be supported as the modules will come alive way later. But it's all reasonable compromises for hardware that wasn't designed to have a dedicated storage in the secure world and support runtime variables sanely.
But the question is fair if we can evolve from this stage here to an in-kernel approach without causing breakages or other headache to distros adopting it (too early). That's why I asked for the roadmap.
Exactly and this is my point as well. I can't see a technical difference other than 'you won't need to launch the supplicant'. The only thing we need to keep in mind is introduce the fallback between the supplicant and the (future) kernel supplicant gracefully. People might still need to run the supplicant for other reasons. But if we design it with the kernel module taking precedence over the supplicant we should be fine.
So since we lived with it a for a few years, I suggest we let it soak a bit and get tested while we try to move the supplicant bits needed over to the kernel. In the meantime patch #4 needs some adjustments, so I'll rethink the supplicant vs kernel module scenario in case I missed something.
Thanks /Ilias
Jan
-- Siemens AG, Technology Competence Center Embedded Linux
On 22.06.23 21:03, Ilias Apalodimas wrote:
On Thu, Jun 22, 2023 at 08:32:44PM +0200, Jan Kiszka wrote:
On 22.06.23 17:04, Ilias Apalodimas wrote:
Hi Jan,
On Thu, 22 Jun 2023 at 17:56, Jan Kiszka jan.kiszka@siemens.com wrote:
On 22.06.23 10:51, Masahisa Kojima 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: 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.
But that is not yet resolving the architectural problem with that userspace daemon dependency. What are the next steps for that now?
We are trying to find some cycles to work on that, however, I don't have a time estimate on that. But the question is different here. Since this addresses the problems distros have wrt to SetVariableRT (even for a limited set of platforms) are we ok pulling this in? I can't think of a technical reason we shouldn't. The supplicant limitations are known and the firrmwareTPM has a similar set of problems.
It will not change we have to do on the distro side because we have to deal not only with the startup issue and StMM but also with fTPM and with shutdown. Only an in-kernel supplicant for RPMB would resolve that according to my understanding.
Exactly and it's worth noting that even that will come with some minor limitations. E.g the randomseed variables set by the efistub currently won't be supported as the modules will come alive way later. But it's all reasonable compromises for hardware that wasn't designed to have a dedicated storage in the secure world and support runtime variables sanely.
My feeling is that such simpler setups will be the minority, simply because eMMCs with RPMBs are standardized, often included anyway, so come "for free".
But the question is fair if we can evolve from this stage here to an in-kernel approach without causing breakages or other headache to distros adopting it (too early). That's why I asked for the roadmap.
Exactly and this is my point as well. I can't see a technical difference other than 'you won't need to launch the supplicant'. The only thing we need to keep in mind is introduce the fallback between the supplicant and the (future) kernel supplicant gracefully. People might still need to run the supplicant for other reasons. But if we design it with the kernel module taking precedence over the supplicant we should be fine.
So since we lived with it a for a few years, I suggest we let it soak a bit and get tested while we try to move the supplicant bits needed over to the kernel. In the meantime patch #4 needs some adjustments, so I'll rethink the supplicant vs kernel module scenario in case I missed something.
Were there distros adopting all this already? I thought this was a privilege of custom integrations where you can evolve things simply in lock-step? At least Debian wasn't considering all these dependencies yet, even though now providing tee-supplicant. We are patching it for now [1].
Jan
[1] https://github.com/BaochengSu/isar/commit/d7646e3bb9d882b26eaf2517fece624010...
[...]
But that is not yet resolving the architectural problem with that userspace daemon dependency. What are the next steps for that now?
We are trying to find some cycles to work on that, however, I don't have a time estimate on that. But the question is different here. Since this addresses the problems distros have wrt to SetVariableRT (even for a limited set of platforms) are we ok pulling this in? I can't think of a technical reason we shouldn't. The supplicant limitations are known and the firrmwareTPM has a similar set of problems.
It will not change we have to do on the distro side because we have to deal not only with the startup issue and StMM but also with fTPM and with shutdown. Only an in-kernel supplicant for RPMB would resolve that according to my understanding.
Exactly and it's worth noting that even that will come with some minor limitations. E.g the randomseed variables set by the efistub currently won't be supported as the modules will come alive way later. But it's all reasonable compromises for hardware that wasn't designed to have a dedicated storage in the secure world and support runtime variables sanely.
My feeling is that such simpler setups will be the minority, simply because eMMCs with RPMBs are standardized, often included anyway, so come "for free".
Yea maybe, I always have the (maybe false) hope that hardware will evolve sanely.
But the question is fair if we can evolve from this stage here to an in-kernel approach without causing breakages or other headache to distros adopting it (too early). That's why I asked for the roadmap.
Exactly and this is my point as well. I can't see a technical difference other than 'you won't need to launch the supplicant'. The only thing we need to keep in mind is introduce the fallback between the supplicant and the (future) kernel supplicant gracefully. People might still need to run the supplicant for other reasons. But if we design it with the kernel module taking precedence over the supplicant we should be fine.
So since we lived with it a for a few years, I suggest we let it soak a bit and get tested while we try to move the supplicant bits needed over to the kernel. In the meantime patch #4 needs some adjustments, so I'll rethink the supplicant vs kernel module scenario in case I missed something.
Were there distros adopting all this already? I thought this was a privilege of custom integrations where you can evolve things simply in lock-step? At least Debian wasn't considering all these dependencies yet, even though now providing tee-supplicant. We are patching it for now [1].
I've been working with Fedora and the OP-TEE community to get some of the pieces in place. As a result, Fedora already compiles the TEE client without RPMB emulation support [0]. We've also fixed the optee-client and removed the compile time dependency of choosing the right RPMB. The device the supplicant now binds to is selectable at runtime [1]. With these two already merged the user-space tee client is hardware agnostic (as it should be).
There's two things missing from distros - Lift the !PSTORE Kconfig limitation this patchset carries so distros can unconditionally enable the module in their builds. But we can do this later while coordinating with distros that build the userspace packages correctly. - Distros needs to scan for the rpmb they want to control in sysfs and launch the supplicant with the appropriate --rpmb-cid option
[0] https://src.fedoraproject.org/rpms/optee_client/blob/rawhide/f/optee_client.... [1] commit 5a69d55d6596 ("tee-supplicant: add --rpmb-cid command line option") in the optee_client repo
Thanks /Ilias
Jan
[1] https://github.com/BaochengSu/isar/commit/d7646e3bb9d882b26eaf2517fece624010...
-- Siemens AG, Technology Competence Center Embedded Linux
op-tee@lists.trustedfirmware.org