- OP-TEE - lists.trustedfirmware.org

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, 1 month

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, 1 month

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, 1 month

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, 1 month

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, 1 month

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, 1 month

2
1
0 0

Linaro OP-TEE Contribution, LOC, monthly meeting September 26

by Jens Wiklander

Hi Today Tuesday, September 26 it's time for another LOC monthly meeting. Sorry for the short notice. For time and connection details see the calendar at https://www.trustedfirmware.org/meetings/ I have a few items for the agenda: - Firmware handoff in OP-TEE https://github.com/OP-TEE/optee_os/pull/6308 - We've started to upstream a few PRs that will require bumping the major version to 4 Any other topics? Thanks, Jens

1 year, 1 month

1
0
0 0

TA access to its own checksum

by Robert Delien

Hi, For legislative purposes, one of our Trusted Applications needs to show a checksum to the end-user on our secure screen, to allow verification. Albeit a bit unconventional use of it, we thought that the TA's own checksum would be ideal for that, but we're having difficulties figuring how to access that, if at all possible. So I have to turn to you guys here: Is there any way for a TA to access its own checksum? And if yes, could somebody please give me some pointers on how to do this? With kind regards, Robert. -- DISCLAIMER De informatie, verzonden in of met dit e-mailbericht, is vertrouwelijk en uitsluitend voor de geadresseerde(n) bestemd. Het gebruik van de informatie in dit bericht, de openbaarmaking, vermenigvuldiging, verspreiding en|of verstrekking daarvan aan derden is niet toegestaan. Gebruik van deze informatie door anderen dan geadresseerde(n) is strikt verboden. Aan deze informatie kunnen geen rechten worden ontleend. U wordt verzocht bij onjuiste adressering de afzender direct te informeren door het bericht te retourneren en het bericht uit uw computersysteem te verwijderen.

1 year, 2 months

3
2
0 0

[PATCH -next] tee: Remove unused declarations

by Yue Haibing

Commit 4fb0a5eb364d ("tee: add OP-TEE driver") declared but never implemented optee_supp_read()/optee_supp_write(). Commit 967c9cca2cc5 ("tee: generic TEE subsystem") never implemented tee_shm_init(). Signed-off-by: Yue Haibing <yuehaibing(a)huawei.com> --- drivers/tee/optee/optee_private.h | 2 -- drivers/tee/tee_private.h | 2 -- 2 files changed, 4 deletions(-) diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index 72685ee0d53f..6bb5cae09688 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -238,8 +238,6 @@ int optee_notif_send(struct optee *optee, u_int key); u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, struct tee_param *param); -int optee_supp_read(struct tee_context *ctx, void __user *buf, size_t len); -int optee_supp_write(struct tee_context *ctx, void __user *buf, size_t len); void optee_supp_init(struct optee_supp *supp); void optee_supp_uninit(struct optee_supp *supp); void optee_supp_release(struct optee_supp *supp); diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h index 409cadcc1cff..754e11dcb240 100644 --- a/drivers/tee/tee_private.h +++ b/drivers/tee/tee_private.h @@ -47,8 +47,6 @@ struct tee_device { struct tee_shm_pool *pool; }; -int tee_shm_init(void); - int tee_shm_get_fd(struct tee_shm *shm); bool tee_device_get(struct tee_device *teedev); -- 2.34.1

1 year, 2 months

3
2
0 0

[GIT PULL] TEE fix for v6.6

by Jens Wiklander

Hello arm-soc maintainers, Please pull this small fix that removes a few unused function declarations in 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/optee-for-for-v6.6 for you to fetch changes up to 069969d6c5264d2348fd6cf0cedc00fd87ff3cee: tee: Remove unused declarations (2023-09-13 08:16:24 +0200) ---------------------------------------------------------------- Remove a few unused declarations in TEE subsystem ---------------------------------------------------------------- Yue Haibing (1): tee: Remove unused declarations drivers/tee/optee/optee_private.h | 2 -- drivers/tee/tee_private.h | 2 -- 2 files changed, 4 deletions(-)

1 year, 2 months

1
0
0 0

2024

2023

2022

2021

2020

OP-TEE