- OP-TEE - lists.trustedfirmware.org

[PATCH v8 0/5] introduce tee-based EFI Runtime Variable Service

by Masahisa Kojima

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/20230609094532.562934-1-ilias.apalodimas@linaro… 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/20230728134832.326467-1-sumit.garg@linaro.org/ 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

1 year, 6 months

5
19
0 0

[PATCH v2] tee: optee: Fix supplicant based device enumeration

by Sumit Garg

Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown: [ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024 Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant. While at it use the global system workqueue for OP-TEE bus scanning work rather than our own custom one. Reported-by: Jan Kiszka <jan.kiszka(a)siemens.com> Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Signed-off-by: Sumit Garg <sumit.garg(a)linaro.org> --- Changes in v2: Apologies for taking it too long push this v2. Actually I did brainstorm how to best fix this tee-supplicant dependent device probing. Its hard to predict the lifetime of user-space daemon from kernel space. So following is the least intrusive approach: - Use device names to seperate out tee-supplicant dependent devices via this patch. - Since user-space service is aware about tee-supplicant lifespan, so allow the user-space service to unbind tee-supplicant dependent devices before killing the supplicant. Following command has to be added to the tee-supplicant service file. $ for dev in /sys/bus/tee/devices/*; do if [[ "$dev" == *"optee-ta-supp-"* ]]; \ then echo $(basename "$dev") > $dev/driver/unbind; fi done drivers/tee/optee/core.c | 13 ++----------- drivers/tee/optee/device.c | 13 ++++++++++--- drivers/tee/optee/optee_private.h | 2 -- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index d01ca47f7bde..8ee3c71bd989 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -15,7 +15,6 @@ #include <linux/string.h> #include <linux/tee_drv.h> #include <linux/types.h> -#include <linux/workqueue.h> #include "optee_private.h" int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, @@ -110,12 +109,7 @@ int optee_open(struct tee_context *ctx, bool cap_memref_null) if (!optee->scan_bus_done) { INIT_WORK(&optee->scan_bus_work, optee_bus_scan); - optee->scan_bus_wq = create_workqueue("optee_bus_scan"); - if (!optee->scan_bus_wq) { - kfree(ctxdata); - return -ECHILD; - } - queue_work(optee->scan_bus_wq, &optee->scan_bus_work); + schedule_work(&optee->scan_bus_work); optee->scan_bus_done = true; } } @@ -159,10 +153,7 @@ void optee_release_supp(struct tee_context *ctx) struct optee *optee = tee_get_drvdata(ctx->teedev); optee_release_helper(ctx, optee_close_session_helper); - if (optee->scan_bus_wq) { - destroy_workqueue(optee->scan_bus_wq); - optee->scan_bus_wq = NULL; - } + optee_supp_release(&optee->supp); } diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); } -static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL; + const char *dev_name_fmt = NULL; int rc; optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL); @@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid) optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device; - if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) { + + if (func == PTA_CMD_GET_DEVICES_SUPP) + dev_name_fmt = "optee-ta-supp-%pUb"; + else + dev_name_fmt = "optee-ta-%pUb"; + + if (dev_set_name(&optee_device->dev, dev_name_fmt, device_uuid)) { kfree(optee_device); return -ENOMEM; } @@ -142,7 +149,7 @@ static int __optee_enumerate_devices(u32 func) num_devices = shm_size / sizeof(uuid_t); for (idx = 0; idx < num_devices; idx++) { - rc = optee_register_device(&device_uuid[idx]); + rc = optee_register_device(&device_uuid[idx], func); if (rc) goto out_shm; } diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index 6dcecb83c893..af4aa266c3fb 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -193,7 +193,6 @@ struct optee_ops { * @pool: shared memory pool * @rpc_param_count: If > 0 number of RPC parameters to make room for * @scan_bus_done flag if device registation was already done. - * @scan_bus_wq workqueue to scan optee bus and register optee drivers * @scan_bus_work workq to scan optee bus and register optee drivers */ struct optee { @@ -212,7 +211,6 @@ struct optee { struct tee_shm_pool *pool; unsigned int rpc_param_count; bool scan_bus_done; - struct workqueue_struct *scan_bus_wq; struct work_struct scan_bus_work; }; -- 2.34.1

1 year, 6 months

4
5
0 0

Preparing for OP-TEE 4.0.0

by Jérôme Forissier

[BCC all OP-TEE maintainers] Hi OP-TEE maintainers & contributors, OP-TEE v4.0.0 is scheduled to be released on 2023-10-20. So, now is a good time to start testing the master branch on the various platforms and report/fix any bugs. The GitHub pull request for collecting Tested-by tags or any other comments is https://github.com/OP-TEE/optee_os/pull/6341. As usual, we will create a release candidate tag one week before the release date for final testing. Note that this is a major version update. We will be merging some pull requests that may break backward compatibility in some (limited) way just before creating the 4.0.0-rc1 tag. These pull requests have "[v4]" in their title and can be found here: https://github.com/OP-TEE/optee_os/pulls?q=is%3Apr+is%3Aopen+in%3Atitle+%5B… In addition to that you can find some additional information related to releases here: https://optee.readthedocs.io/en/latest/general/releases.html Regards, -- Jerome Forissier

1 year, 6 months

1
1
0 0

[PATCH v10 0/4] tee: introduce TEE system sssion

by Etienne Carriere

This series introduces TEE system sessions for TEE service sessions that require TEE to provision resources to prevent deadlock when clients call the TEE. This deadlock situation can happen when a TEE service is used by low level system resources as for example when Linux kernel uses SCMI service embedded in TEE for clock, reset, regulator, etc... controls. This case is detailled in patch 2/4: > This feature is needed to prevent a system deadlock when several TEE > client applications invoke TEE, consuming all TEE thread contexts > available in the secure world. The deadlock can happen in the OP-TEE > driver for example if all these TEE threads issue an RPC call from TEE > to Linux OS to access an eMMC RPMB partition (TEE secure storage) which > device clock or regulator controller is accessed through an OP-TEE SCMI > services. In that case, Linux SCMI driver must reach OP-TEE SCMI > service without waiting until one of the consumed TEE threads is freed. Etienne Carriere (4): tee: optee: system call property tee: system session tee: optee: support tracking system threads firmware: arm_scmi: optee: use optee system invocation drivers/firmware/arm_scmi/optee.c | 4 + drivers/tee/optee/call.c | 152 +++++++++++++++++++++++++++--- drivers/tee/optee/core.c | 5 +- drivers/tee/optee/ffa_abi.c | 13 +-- drivers/tee/optee/optee_private.h | 33 ++++++- drivers/tee/optee/smc_abi.c | 31 ++++-- drivers/tee/tee_core.c | 8 ++ include/linux/tee_drv.h | 16 ++++ 8 files changed, 227 insertions(+), 35 deletions(-) -- 2.25.1

1 year, 6 months

4
13
0 0

[PATCH] tee: make tee_class constant

by Greg Kroah-Hartman

Now that the driver core allows for struct class to be in read-only memory, we should make all 'class' structures declared at build time placing them into read-only memory, instead of having to be dynamically allocated at runtime. Cc: Jens Wiklander <jens.wiklander(a)linaro.org> Cc: Sumit Garg <sumit.garg(a)linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/tee/tee_core.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 0eb342de0b00..5ddfd5d9ac7f 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -40,7 +40,10 @@ static const uuid_t tee_client_uuid_ns = UUID_INIT(0x58ac9ca0, 0x2086, 0x4683, static DECLARE_BITMAP(dev_mask, TEE_NUM_DEVICES); static DEFINE_SPINLOCK(driver_lock); -static struct class *tee_class; +static const struct class tee_class = { + .name = "tee", +}; + static dev_t tee_devt; struct tee_context *teedev_open(struct tee_device *teedev) @@ -919,7 +922,7 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc, teedesc->flags & TEE_DESC_PRIVILEGED ? "priv" : "", teedev->id - offs); - teedev->dev.class = tee_class; + teedev->dev.class = &tee_class; teedev->dev.release = tee_release_device; teedev->dev.parent = dev; @@ -1112,7 +1115,7 @@ tee_client_open_context(struct tee_context *start, dev = &start->teedev->dev; do { - dev = class_find_device(tee_class, dev, &match_data, match_dev); + dev = class_find_device(&tee_class, dev, &match_data, match_dev); if (!dev) { ctx = ERR_PTR(-ENOENT); break; @@ -1226,10 +1229,10 @@ static int __init tee_init(void) { int rc; - tee_class = class_create("tee"); - if (IS_ERR(tee_class)) { + rc = class_register(&tee_class); + if (rc) { pr_err("couldn't create class\n"); - return PTR_ERR(tee_class); + return rc; } rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee"); @@ -1249,8 +1252,7 @@ static int __init tee_init(void) out_unreg_chrdev: unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES); out_unreg_class: - class_destroy(tee_class); - tee_class = NULL; + class_unregister(&tee_class); return rc; } @@ -1259,8 +1261,7 @@ static void __exit tee_exit(void) { bus_unregister(&tee_bus_type); unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES); - class_destroy(tee_class); - tee_class = NULL; + class_unregister(&tee_class); } subsys_initcall(tee_init); -- 2.42.0

1 year, 6 months

2
2
0 0

[PATCH v3] virtio-tee: Reserve device ID 46 for TEE device

by jeshwank

In a virtual environment, an application running in guest VM may want to delegate security sensitive tasks to a Trusted Application (TA) running within a Trusted Execution Environment (TEE). A TEE is a trusted OS running in some secure environment, for example, TrustZone on ARM CPUs, or a separate secure co-processor etc. A virtual TEE device emulates a TEE within a guest VM. Such a virtual TEE device supports multiple operations such as: VIRTIO_TEE_CMD_OPEN_DEVICE – Open a communication channel with virtio TEE device. VIRTIO_TEE_CMD_CLOSE_DEVICE – Close communication channel with virtio TEE device. VIRTIO_TEE_CMD_GET_VERSION – Get version of virtio TEE. VIRTIO_TEE_CMD_OPEN_SESSION – Open a session to communicate with trusted application running in TEE. VIRTIO_TEE_CMD_CLOSE_SESSION – Close a session to end communication with trusted application running in TEE. VIRTIO_TEE_CMD_INVOKE_FUNC – Invoke a command or function in trusted application running in TEE. VIRTIO_TEE_CMD_CANCEL_REQ – Cancel an ongoing command within TEE. VIRTIO_TEE_CMD_REGISTER_MEM - Register shared memory with TEE. VIRTIO_TEE_CMD_UNREGISTER_MEM - Unregister shared memory from TEE. We would like to reserve device ID 46 for Virtio-TEE device. Signed-off-by: Jeshwanth Kumar <jeshwanthkumar.nk(a)amd.com> Reviewed-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com> Reviewed-by: Parav Pandit <parav(a)nvidia.com> Acked-by: Sumit Garg <sumit.garg(a)linaro.org> --- content.tex | 2 ++ 1 file changed, 2 insertions(+) diff --git a/content.tex b/content.tex index 0a62dce..644aa4a 100644 --- a/content.tex +++ b/content.tex @@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types} \hline 45 & SPI master \\ \hline +46 & TEE device \\ +\hline \end{tabular} Some of the devices above are unspecified by this document, -- 2.25.1

1 year, 6 months

2
1
0 0

[GIT PULL] AMDTEE fix for v6.6

by Jens Wiklander

Hello arm-soc maintainers, Please pull this small AMDTEE driver fix addressing a possible user-after-free vulnerability. Note that this isn't a usual Arm driver update. This targets AMD instead, but is part of the TEE subsystem. Thanks, Jens The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c: Linux 6.5 (2023-08-27 14:49:51 -0700) are available in the Git repository at: https://git.linaro.org/people/jens.wiklander/linux-tee.git/ tags/amdtee-fix-for-v6.6 for you to fetch changes up to f4384b3e54ea813868bb81a861bf5b2406e15d8f: tee: amdtee: fix use-after-free vulnerability in amdtee_close_session (2023-10-03 19:13:53 +0200) ---------------------------------------------------------------- AMDTEE fix possible use-after-free ---------------------------------------------------------------- Rijo Thomas (1): tee: amdtee: fix use-after-free vulnerability in amdtee_close_session drivers/tee/amdtee/core.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)

1 year, 6 months

1
0
0 0

[PATCH v2 1/1] tee: amdtee: fix use-after-free vulnerability in amdtee_close_session

by Rijo Thomas

There is a potential race condition in amdtee_close_session that may cause use-after-free in amdtee_open_session. For instance, if a session has refcount == 1, and one thread tries to free this session via: kref_put(&sess->refcount, destroy_session); the reference count will get decremented, and the next step would be to call destroy_session(). However, if in another thread, amdtee_open_session() is called before destroy_session() has completed execution, alloc_session() may return 'sess' that will be freed up later in destroy_session() leading to use-after-free in amdtee_open_session. To fix this issue, treat decrement of sess->refcount and removal of 'sess' from session list in destroy_session() as a critical section, so that it is executed atomically. Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Cc: stable(a)vger.kernel.org Signed-off-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com> --- v2: * Introduced kref_put_mutex() as suggested by Sumit Garg. drivers/tee/amdtee/core.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 372d64756ed6..3c15f6a9e91c 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -217,12 +217,12 @@ static int copy_ta_binary(struct tee_context *ctx, void *ptr, void **ta, return rc; } +/* mutex must be held by caller */ static void destroy_session(struct kref *ref) { struct amdtee_session *sess = container_of(ref, struct amdtee_session, refcount); - mutex_lock(&session_list_mutex); list_del(&sess->list_node); mutex_unlock(&session_list_mutex); kfree(sess); @@ -272,7 +272,8 @@ int amdtee_open_session(struct tee_context *ctx, if (arg->ret != TEEC_SUCCESS) { pr_err("open_session failed %d\n", arg->ret); handle_unload_ta(ta_handle); - kref_put(&sess->refcount, destroy_session); + kref_put_mutex(&sess->refcount, destroy_session, + &session_list_mutex); goto out; } @@ -290,7 +291,8 @@ int amdtee_open_session(struct tee_context *ctx, pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS); handle_close_session(ta_handle, session_info); handle_unload_ta(ta_handle); - kref_put(&sess->refcount, destroy_session); + kref_put_mutex(&sess->refcount, destroy_session, + &session_list_mutex); rc = -ENOMEM; goto out; } @@ -331,7 +333,7 @@ int amdtee_close_session(struct tee_context *ctx, u32 session) handle_close_session(ta_handle, session_info); handle_unload_ta(ta_handle); - kref_put(&sess->refcount, destroy_session); + kref_put_mutex(&sess->refcount, destroy_session, &session_list_mutex); return 0; } -- 2.25.1

1 year, 6 months

3
2
0 0

[PATCH 1/1] tee: amdtee: fix use-after-free vulnerability in amdtee_close_session

by Rijo Thomas

There is a potential race condition in amdtee_close_session that may cause use-after-free in amdtee_open_session. For instance, if a session has refcount == 1, and one thread tries to free this session via: kref_put(&sess->refcount, destroy_session); the reference count will get decremented, and the next step would be to call destroy_session(). However, if in another thread, amdtee_open_session() is called before destroy_session() has completed execution, alloc_session() may return 'sess' that will be freed up later in destroy_session() leading to use-after-free in amdtee_open_session. To fix this issue, treat decrement of sess->refcount and invocation of destroy_session() as a single critical section, so that it is executed atomically. Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Cc: stable(a)vger.kernel.org Signed-off-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com> --- drivers/tee/amdtee/core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 372d64756ed6..04cee03bec9d 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -217,14 +217,13 @@ static int copy_ta_binary(struct tee_context *ctx, void *ptr, void **ta, return rc; } +/* mutex must be held by caller */ static void destroy_session(struct kref *ref) { struct amdtee_session *sess = container_of(ref, struct amdtee_session, refcount); - mutex_lock(&session_list_mutex); list_del(&sess->list_node); - mutex_unlock(&session_list_mutex); kfree(sess); } @@ -272,7 +271,9 @@ int amdtee_open_session(struct tee_context *ctx, if (arg->ret != TEEC_SUCCESS) { pr_err("open_session failed %d\n", arg->ret); handle_unload_ta(ta_handle); + mutex_lock(&session_list_mutex); kref_put(&sess->refcount, destroy_session); + mutex_unlock(&session_list_mutex); goto out; } @@ -290,7 +291,9 @@ int amdtee_open_session(struct tee_context *ctx, pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS); handle_close_session(ta_handle, session_info); handle_unload_ta(ta_handle); + mutex_lock(&session_list_mutex); kref_put(&sess->refcount, destroy_session); + mutex_unlock(&session_list_mutex); rc = -ENOMEM; goto out; } @@ -331,7 +334,9 @@ int amdtee_close_session(struct tee_context *ctx, u32 session) handle_close_session(ta_handle, session_info); handle_unload_ta(ta_handle); + mutex_lock(&session_list_mutex); kref_put(&sess->refcount, destroy_session); + mutex_unlock(&session_list_mutex); return 0; } -- 2.25.1

1 year, 6 months

2
2
0 0

Re: [virtio-dev] [PATCH v2] virtio-tee: Reserve device ID 46 for TEE device

by Sumit Garg

+cc OP-TEE ML On Tue, 26 Sept 2023 at 13:47, Rijo Thomas <Rijo-john.Thomas(a)amd.com> wrote: > > On 9/26/2023 1:19 PM, Sumit Garg wrote: > > On Tue, 26 Sept 2023 at 12:53, Rijo Thomas <Rijo-john.Thomas(a)amd.com> wrote: > >> > >> On 9/26/2023 12:14 PM, Sumit Garg wrote: > >>> +cc Alex > >>> > >>> On Tue, 26 Sept 2023 at 08:16, Jens Wiklander <jens.wiklander(a)linaro.org> wrote: > >>>> > >>>> Hi, > >>>> > >>>> [+cc Arnd] > >>>> > >>>> On Tue, Sep 26, 2023 at 8:00 AM Sumit Garg <sumit.garg(a)linaro.org> wrote: > >>>>> > >>>>> +cc Jens > >>>>> > >>>>>> In a virtual environment, an application running in guest VM may want > >>>>>> to delegate security sensitive tasks to a Trusted Application (TA) > >>>>>> running within a Trusted Execution Environment (TEE). A TEE is a trusted > >>>>>> OS running in some secure environment, for example, TrustZone on ARM > >>>>>> CPUs, or a separate secure co-processor etc. > >>>>> > >>>>> I have been exploring this area quite recently with an effort to have a common VIRIO interface which can support different trusted OS implementations. I guess you intend to test it with AMD-TEE, right? Any plans to test it with OP-TEE? As currently we have these two supported upstream. > >>>>> > >> Yes, we have tested with AMD-TEE. We have not yet tested with OP-TEE. Sure, we will try it out. > > > > Glad to hear that. I can help get it tested with OP-TEE as well. > > > > We will test it out internally. Shall let you know in case we need help. > > >> > >>>>> Do you currently have any virtio frontend/backend implementations for this? > >>>>> > >> > >> Yes, we have. Frontend is a Linux virtio-TEE driver, and backend is virtio-TEE device emulated in QEMU. > >> We used the Xen hypervisor. > > > > Can you share corresponding references? I can give it a try using Qemu with KVM. > > > > We will share it in next couple of weeks. We have not yet hosted the code for external consumption. > > >> > >>>>>> > >>>>>> A virtual TEE device emulates a TEE within a guest VM. Such a virtual > >>>>>> TEE device supports multiple operations such as: > >>>>>> > >>>>>> VIRTIO_TEE_CMD_OPEN_DEVICE – Open a communication channel with virtio > >>>>>> TEE device. > >>>>>> VIRTIO_TEE_CMD_CLOSE_DEVICE – Close communication channel with virtio > >>>>>> TEE device. > >>>>>> VIRTIO_TEE_CMD_GET_VERSION – Get version of virtio TEE. > >>>>>> VIRTIO_TEE_CMD_OPEN_SESSION – Open a session to communicate with > >>>>>> trusted application running in TEE. > >>>>>> VIRTIO_TEE_CMD_CLOSE_SESSION – Close a session to end communication > >>>>>> with trusted application running in TEE. > >>>>>> VIRTIO_TEE_CMD_INVOKE_FUNC – Invoke a command or function in trusted > >>>>>> application running in TEE. > >>>>>> VIRTIO_TEE_CMD_CANCEL_REQ – Cancel an ongoing command within TEE. > >>>>>> > >>>>> > >>>>> How about shared memory support? We would like to register guest pages with the trusted OS. > >> We have a command VIRTIO_TEE_CMD_REGISTER_MEM for registering shared memory buffer with Trusted OS. > > > > I suppose the commit message has to be appended then. Do you have the > > draft virtio-tee device specification ready for review? I would be > > interested to review that. > > > > Yes, the command is missed out in the commit message. With the commit message updated to include support for shared memory, feel free to add: Acked-by: Sumit Garg <sumit.garg(a)linaro.org> > > We are in the process of preparing virtio-tee device specification. We will be sending it out to this > list. I would also suggest you to CC: op-tee(a)lists.trustedfirmware.org for review. > > >> > >> In this command, the guest pages are copied into a shadow buffer in the host OS. And this shadow > >> buffer is mapped with Trusted OS. So, buffer-copy is involved. > >> > >> One limitation, that we had was that the guest pages were non-contiguous. So, the number of physical > >> pages that had to be mapped with Trusted OS was exceeding 64 entries when we were testing out the > >> registering of guest pages. AMD-TEE Trusted OS can map a physically non-contiguous buffer, but the > >> number of sg entries for such a buffer must be less than 64. So, we resorted to using a shadow buffer > >> that is allocated within host, and gets mapped with Trusted OS. > > > > I don't think OP-TEE OS has such a limitation on non-contiguous pages. > > So I would suggest you to keep VIRTIO_TEE_CMD_REGISTER_MEM as part of > > the ABI. It can be an optional feature for a particular trusted OS > > implementation to support. > > > > Currently, the reg_mem (register memory) control is dictated by a flag in virtio-tee qemu code. This flag > for our testing was hard-coded as false. We will enhance our code, so that it is configurable. The value > of reg_mem shall be set to true/false depending upon whether the underlying TEE driver reports TEE_GEN_CAP_REG_MEM. Sounds good to me. -Sumit > > Thanks, > Rijo > > > -Sumit > > > >> > >> Thanks, > >> Rijo > >> > >>>> > >>>> Coincidently Arnd and I (among others) discussed this in person last > >>>> week and the conclusion was that only temporary shared memory is > >>>> possible with virtio. So the shared memory has to be set up and torn > >>>> down by the host during each operation, typically open-session or > >>>> invoke-func. > >>> > >>> Agree as I was part of those discussions. But I would like to > >>> understand the reasoning behind it. Is there any restriction by VIRTIO > >>> specification that we can't register guest page PAs to a device (TEE > >>> in our case) to allow for zero copy transfers? > >>> > >>> Alex mentioned some references to virtio GPU device. I suppose I need > >>> to dive into its implementation to see if there are any similarities > >>> to our use-case. > >>> > >>>> That might not be optimal if trying to maximize > >>>> performance, but it is portable. > >>> > >>> IMO, the ABI should be flexible enough to support a TEE with optimum > >>> performance. > >>> > >>> -Sumit > >>> > >>>> > >>>> Cheers, > >>>> Jens > >>>> > >>>>> > >>>>> -Sumit > >>>>> > >>>>>> We would like to reserve device ID 46 for Virtio-TEE device. > >>>>>> > >>>>>> Signed-off-by: Jeshwanth Kumar <jeshwanthkumar.nk(a)amd.com> > >>>>>> --- > >>>>>> content.tex | 2 ++ > >>>>>> 1 file changed, 2 insertions(+) > >>>>>> > >>>>>> diff --git a/content.tex b/content.tex > >>>>>> index 0a62dce..644aa4a 100644 > >>>>>> --- a/content.tex > >>>>>> +++ b/content.tex > >>>>>> @@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types} > >>>>>> \hline > >>>>>> 45 & SPI master \\ > >>>>>> \hline > >>>>>> +46 & TEE device \\ > >>>>>> +\hline > >>>>>> \end{tabular} > >>>>>> > >>>>>> Some of the devices above are unspecified by this document,

1 year, 6 months

2
1
0 0

2025

2024

2023

2022

2021

2020

OP-TEE