- OP-TEE - lists.trustedfirmware.org

by Jens Wiklander

[BCC all OP-TEE maintainers] Hi OP-TEE maintainers & contributors, OP-TEE version 4.10.0 is now scheduled for release on 2026-04-17. So, now is a good time to start testing the master branch across various platforms and report or 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/7753. We will create the release candidate tag 4.10.0-rc1 this Friday, April 3. You can find more information related to releases here: https://optee.readthedocs.io/en/latest/general/releases.html Thanks, Jens

1 month, 2 weeks

1
1
0 0

OP-TEE: memory corruption in scmi_pin_control_list_associations_handler()

by Dan Carpenter

When we do SCMI on OP-TEE, the in buffer and out buffer are the same buffer. I added some debug code to confirm this: diff --git a/module/msg_smt/src/mod_msg_smt.c b/module/msg_smt/src/mod_msg_smt.c index 853fe66b8d27..1e92dcd49c29 100644 --- a/module/msg_smt/src/mod_msg_smt.c +++ b/module/msg_smt/src/mod_msg_smt.c @@ -175,6 +175,11 @@ static int smt_write_payload(fwk_id_t channel_id, if (!channel_ctx->locked) return FWK_E_ACCESS; + FWK_LOG_ERR("OUT=%p IN=%p %s", + channel_ctx->out->payload, + channel_ctx->in->payload, + (channel_ctx->out->payload == channel_ctx->in->payload) ? "equal" : "different"); + memcpy(((uint8_t*)channel_ctx->out->payload) + offset, payload, size); return FWK_SUCCESS; And it's true: [ 0.000000] OUT=0x9c401004 IN=0x9c401004 equal This normally isn't a problem because we read a few inputs at the start of the function and then write out the result to the output at the end. But in the scmi_pin_control_list_associations_handler() it does a loop where each iteration reads from parameters->index which is input and writes to the output with scmi_pin_control_ctx.scmi_api->write_payload() and that corrupts the input data. Copying the input buffer to a the stack the issue for me, but I feel like it is a hack. It would be better to use separate buffers for input and output. I think this comes from: https://github.com/OP-TEE/optee_os/blob/master/core/kernel/pseudo_ta.c#L93 I added a print statement to there as well: diff --git a/core/kernel/pseudo_ta.c b/core/kernel/pseudo_ta.c index 587faa41a770..426870fb934c 100644 --- a/core/kernel/pseudo_ta.c +++ b/core/kernel/pseudo_ta.c @@ -90,6 +90,7 @@ static TEE_Result copy_in_param(struct ts_session *s __maybe_unused, va = NULL; } + EMSG("n=%lu va=%p", n, va); tee_param[n].memref.buffer = va; tee_param[n].memref.size = mem->size; break; E/TC:? 0 copy_in_param:93 n=1 va=0x9c401000 E/TC:? 0 copy_in_param:93 n=2 va=0x9c401000 Here is the patch to save "parameters" to a different buffer. --- .../scmi_pin_control/src/mod_scmi_pin_control.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/module/scmi_pin_control/src/mod_scmi_pin_control.c b/module/scmi_pin_control/src/mod_scmi_pin_control.c index a0b90dd2b73f..54e613b70f69 100644 --- a/module/scmi_pin_control/src/mod_scmi_pin_control.c +++ b/module/scmi_pin_control/src/mod_scmi_pin_control.c @@ -344,7 +344,7 @@ static int scmi_pin_control_list_associations_handler( fwk_id_t service_id, const uint32_t *payload) { - const struct scmi_pin_control_list_associations_a2p *parameters; + const struct scmi_pin_control_list_associations_a2p parameters; uint32_t payload_size; uint16_t identifiers_count; uint16_t total_number_of_associations; @@ -362,8 +362,9 @@ static int scmi_pin_control_list_associations_handler( payload_size = (uint32_t)sizeof(return_values); parameters = (const struct scmi_pin_control_list_associations_a2p *)payload; + memcpy(&parameters, payload, sizeof(parameters)); - status = map_identifier(parameters->identifier, &mapped_identifier); + status = map_identifier(parameters.identifier, &mapped_identifier); if (status != FWK_SUCCESS) { return_values.status = SCMI_NOT_FOUND; @@ -371,7 +372,7 @@ static int scmi_pin_control_list_associations_handler( } status = scmi_pin_control_ctx.pinctrl_api->get_total_number_of_associations( - mapped_identifier, parameters->flags, &total_number_of_associations); + mapped_identifier, parameters.flags, &total_number_of_associations); if (status != FWK_SUCCESS) { return_values.status = SCMI_NOT_FOUND; goto exit; @@ -388,11 +389,11 @@ static int scmi_pin_control_list_associations_handler( identifiers_count = (uint16_t)FWK_MIN( buffer_allowed_identifiers, - (uint16_t)(total_number_of_associations - parameters->index)); + (uint16_t)(total_number_of_associations - parameters.index)); return_values.flags = identifiers_count; return_values.flags |= SHIFT_LEFT_BY_POS( - (total_number_of_associations - parameters->index - identifiers_count), + (total_number_of_associations - parameters.index - identifiers_count), NUM_OF_REMAINING_ELEMENTS_POS); for (identifier_index = 0; identifier_index < identifiers_count; @@ -401,8 +402,8 @@ static int scmi_pin_control_list_associations_handler( status = scmi_pin_control_ctx.pinctrl_api->get_list_associations( mapped_identifier, - parameters->flags, - (parameters->index + identifier_index), + parameters.flags, + (parameters.index + identifier_index), &object_id); if (status != FWK_SUCCESS) { return_values.status = SCMI_NOT_FOUND; -- 2.51.0

1 month, 3 weeks

2
3
0 0

Possible race during OP-TEE kernel module device probing

by Shf Chen (陳少甫)

Hi, We found a possible race condition issue during OP-TEE kernel driver probing the device. A NULL pointer dereference exception can happen when another kernel driver open OP-TEE context with tee_client_open_context() then do a SMC call to OP-TEE. Below is the exception: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 Mem abort info: ESR = 0x0000000096000005 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x05: level 1 translation fault Data abort info: ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000 CM = 0, WnR = 0, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 user pgtable: 4k pages, 39-bit VAs, pgdp=00000001026bb000 [0000000000000000] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000 Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP Workqueue: events_unbound deferred_probe_work_func pstate: 03400005 (nzcv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--) pc : optee_cq_wait_init+0x78/0x124 [optee] lr : optee_cq_wait_init+0x60/0x124 [optee] sp : ffffffc081fcb7f0 x29: ffffffc081fcb7f0 x28: 0000000000000000 x27: 0000000000001000 x26: ffffff8080e42c60 x25: ffffff8084d46040 x24: 0000000000000000 x23: 0000000000000000 x22: ffffffc081fcb8c0 x21: ffffffc081fcb8a8 x20: 0000000000000000 x19: ffffff8082741570 x18: ffffffe572f8ca00 x17: 00000000fa28650f x16: 00000000fa28650f x15: ffffff8084d47000 x14: 0000000000000000 x13: 0000000000000000 x12: 0000000084d47000 x11: 0000000000000000 x10: 0000000032000012 x9 : 04d4600000000001 x8 : ffffffc081fcb8c8 x7 : 0000000000000000 x6 : 000000000000003f x5 : ffffff83c86649e0 x4 : 0000000000000008 x3 : 0000000000000000 x2 : ffffff80827415a0 x1 : 0000000000000000 x0 : ffffffc081fcb8c0 Call trace: optee_cq_wait_init+0x78/0x124 [optee f6dbc35f8d96acbe1f3c329d72018151d796208d] optee_smc_do_call_with_arg+0x12c/0x95c [optee f6dbc35f8d96acbe1f3c329d72018151d796208d] optee_shm_register+0x284/0x360 [optee f6dbc35f8d96acbe1f3c329d72018151d796208d] register_shm_helper+0x1a4/0x2f4 [tee cdd8a0d077d984bda1f31f9e8903836edbe46603] tee_shm_register_kernel_buf+0x60/0x90 [tee cdd8a0d077d984bda1f31f9e8903836edbe46603] cmdq_sec_allocate_wsm+0x58/0xc4 [mtk_cmdq_sec_mbox ac82dc958e32252a21c6ce55bb8839ebb288387a] cmdq_sec_probe+0x80/0x4a0 [mtk_cmdq_sec_mbox ac82dc958e32252a21c6ce55bb8839ebb288387a] We found the optee->call_queue hasn't been initialized when our driver called OP-TEE, and it might have some issues about the device data structure initialize order in optee_probe()[drivers/tee/optee/smc_abi.c]: --- rc = tee_device_register(optee->teedev); // <----- TEE device register here if (rc) goto err_unreg_supp_teedev; rc = tee_device_register(optee->supp_teedev); if (rc) goto err_unreg_supp_teedev; optee_cq_init(&optee->call_queue, thread_count); // <----- Some data structures are initialized afterwards optee_supp_init(&optee->supp); optee->smc.memremaped_shm = memremaped_shm; optee->pool = pool; optee_shm_arg_cache_init(optee, arg_cache_flags); mutex_init(&optee->rpmb_dev_mutex); --- We want to ask if the data structure initialization should be done before the tee device registration? Best regards, Shao-Fu Chen

1 month, 3 weeks

3
4
0 0

[PATCH v2 00/15] firmware: qcom: Add OP-TEE PAS service support

by Sumit Garg

From: Sumit Garg <sumit.garg(a)oss.qualcomm.com> Qcom platforms has the legacy of using non-standard SCM calls splintered over the various kernel drivers. These SCM calls aren't compliant with the standard SMC calling conventions which is a prerequisite to enable migration to the FF-A specifications from Arm. OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't support these non-standard SCM calls. And even for newer architectures with S-EL2 and Hafnium support, QTEE won't be able to support SCM calls either with FF-A requirements coming in. And with both OP-TEE and QTEE drivers well integrated in the TEE subsystem, it makes further sense to reuse the TEE bus client drivers infrastructure. The added benefit of TEE bus infrastructure is that there is support for discoverable/enumerable services. With that client drivers don't have to manually invoke a special SCM call to know the service status. So enable the generic Peripheral Authentication Service (PAS) provided by the firmware. It acts as the common layer with different TZ backends plugged in whether it's an SCM implementation or a proper TEE bus based PAS service implementation. The TEE PAS service ABI is designed to be extensible with additional API as PTA_QCOM_PAS_CAPABILITIES. This allows to accommodate any future extensions of the PAS service needed while still maintaining backwards compatibility. Currently OP-TEE support is being added to provide the backend PAS service implementation which can be found as part of this PR [1]. This implementation has been tested on Kodiak/RB3Gen2 board with lemans EVK board being the next target. In addition to that WIN/IPQ targets planning to use OP-TEE will use this service too. Surely the backwards compatibility is maintained and tested for SCM backend. Patch summary: - Patch #1: adds Kodiak EL2 overlay since boot stack with TF-A/OP-TEE only allow UEFI and Linux to boot in EL2. - Patch #2: adds generic PAS service. - Patch #3: migrates SCM backend to generic PAS service. - Patch #4: adds TEE/OP-TEE backend for generic PAS service. - Patch #5-#13: migrates all client drivers to generic PAS service. - Patch #14: drops legacy PAS SCM exported APIs. The patch-set is based on v7.0-rc2 tag and can be found in git tree here [2]. Merge strategy: It is expected due to APIs dependency, the entire patch-set to go via the Qcom tree. All other subsystem maintainers, it will be great if I can get acks for the corresponding subsystem patches. [1] https://github.com/OP-TEE/optee_os/pull/7721 [2] https://git.kernel.org/pub/scm/linux/kernel/git/sumit.garg/linux.git/log/?h… --- Changes in v2: - Fixed kernel doc warnings. - Polish commit message and comments for patch #2. - Pass proper PAS ID in set_remote_state API for media firmware drivers. - Added Maintainer entry and dropped MODULE_AUTHOR. - Picked up ack from Jeff. Mukesh Ojha (1): arm64: dts: qcom: kodiak: Add EL2 overlay Sumit Garg (14): firmware: qcom: Add a generic PAS service firmware: qcom_scm: Migrate to generic PAS service firmware: qcom: Add a PAS TEE service remoteproc: qcom_q6v5_pas: Switch over to generic PAS TZ APIs remoteproc: qcom_q6v5_mss: Switch to generic PAS TZ APIs soc: qcom: mdtloader: Switch to generic PAS TZ APIs remoteproc: qcom_wcnss: Switch to generic PAS TZ APIs remoteproc: qcom: Select QCOM_PAS_TEE service backend drm/msm: Switch to generic PAS TZ APIs media: qcom: Switch to generic PAS TZ APIs net: ipa: Switch to generic PAS TZ APIs wifi: ath12k: Switch to generic PAS TZ APIs firmware: qcom_scm: Remove SCM PAS wrappers MAINTAINERS: Add maintainer entry for Qualcomm PAS TZ service MAINTAINERS | 9 + arch/arm64/boot/dts/qcom/Makefile | 2 + arch/arm64/boot/dts/qcom/kodiak-el2.dtso | 35 ++ drivers/firmware/qcom/Kconfig | 18 + drivers/firmware/qcom/Makefile | 2 + drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++ drivers/firmware/qcom/qcom_pas.h | 53 ++ drivers/firmware/qcom/qcom_pas_tee.c | 477 ++++++++++++++++++ drivers/firmware/qcom/qcom_scm.c | 304 ++++------- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 +- .../media/platform/qcom/iris/iris_firmware.c | 9 +- drivers/media/platform/qcom/venus/firmware.c | 11 +- drivers/net/ipa/ipa_main.c | 13 +- drivers/net/wireless/ath/ath12k/ahb.c | 8 +- drivers/remoteproc/Kconfig | 1 + drivers/remoteproc/qcom_q6v5_mss.c | 5 +- drivers/remoteproc/qcom_q6v5_pas.c | 51 +- drivers/remoteproc/qcom_wcnss.c | 12 +- drivers/soc/qcom/mdt_loader.c | 12 +- include/linux/firmware/qcom/qcom_pas.h | 41 ++ include/linux/firmware/qcom/qcom_scm.h | 29 -- include/linux/soc/qcom/mdt_loader.h | 6 +- 23 files changed, 1108 insertions(+), 303 deletions(-) create mode 100644 arch/arm64/boot/dts/qcom/kodiak-el2.dtso create mode 100644 drivers/firmware/qcom/qcom_pas.c create mode 100644 drivers/firmware/qcom/qcom_pas.h create mode 100644 drivers/firmware/qcom/qcom_pas_tee.c create mode 100644 include/linux/firmware/qcom/qcom_pas.h -- 2.51.0

1 month, 3 weeks

6
40
0 0

[GIT PULL] OP-TEE update for 7.1

by Jens Wiklander

Hello soc maintainers, Please pull this small patch simlifying the OP-TEE context matching logic. Thanks, Jens The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f: Linux 7.0-rc1 (2026-02-22 13:18:59 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git tags/optee-for-v7.1 for you to fetch changes up to 8c6e843f1c26a0b12720cab02f785c46450b6adc: optee: simplify OP-TEE context match (2026-03-04 08:33:31 +0100) ---------------------------------------------------------------- OP-TEE update for 7.1 Simplify TEE implementor ID match logic ---------------------------------------------------------------- Rouven Czerwinski (1): optee: simplify OP-TEE context match drivers/tee/optee/device.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)

1 month, 3 weeks

1
0
0 0

[GIT PULL] TEE update for 7.1

by Jens Wiklander

Hello soc maintainers, Please pull this small patch cleaning up kernel-doc comments in tee_core.h. Thanks, Jens The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f: Linux 7.0-rc1 (2026-02-22 13:18:59 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git tags/tee-for-v7.1 for you to fetch changes up to e416e7fa417b2d2604c1526a2f9cc38da7ced166: tee: clean up tee_core.h kernel-doc (2026-03-13 08:22:54 +0100) ---------------------------------------------------------------- TEE update for 7.1 Clean up tee_core.h kernel-doc to eliminate build warnings ---------------------------------------------------------------- Randy Dunlap (1): tee: clean up tee_core.h kernel-doc include/linux/tee_core.h | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)

1 month, 3 weeks

1
0
0 0

[PATCH 00/14] firmware: qcom: Add OP-TEE PAS service support

by Sumit Garg

From: Sumit Garg <sumit.garg(a)oss.qualcomm.com> Qcom platforms has the legacy of using non-standard SCM calls splintered over the various kernel drivers. These SCM calls aren't compliant with the standard SMC calling conventions which is a prerequisite to enable migration to the FF-A specifications from Arm. OP-TEE as an alternative trusted OS to QTEE can't support these non- standard SCM calls. And even for newer architectures QTEE won't be able to support SCM calls either with FF-A requirements coming in. And with both OP-TEE and QTEE drivers well integrated in the TEE subsystem, it makes further sense to reuse the TEE bus client drivers infrastructure. The added benefit of TEE bus infrastructure is that there is support for discoverable/enumerable services. With that client drivers don't have to manually invoke a special SCM call to know the service status. So enable the generic Peripheral Authentication Service (PAS) provided by the firmware. It acts as the common layer with different TZ backends plugged in whether it's an SCM implementation or a proper TEE bus based PAS service implementation. The TEE PAS service ABI is designed to be extensible with additional API as PTA_QCOM_PAS_CAPABILITIES. This allows to accommodate any future extensions of the PAS service needed while still maintaining backwards compatibility. Currently OP-TEE support is being added to provide the backend PAS service implementation which can be found as part of this PR [1]. This implementation has been tested on Kodiak/RB3Gen2 board with lemans EVK board being the next target. In addition to that WIN/IPQ targets planning to use OP-TEE will use this service too. Patch summary: - Patch #1: adds Kodiak EL2 overlay since boot stack with TF-A/OP-TEE only allow UEFI and Linux to boot in EL2. - Patch #2: adds generic PAS service. - Patch #3: migrates SCM backend to generic PAS service. - Patch #4: adds TEE/OP-TEE backend for generic PAS service. - Patch #5-#13: migrates all client drivers to generic PAS service. - Patch #14: drops legacy PAS SCM exported APIs. The patch-set is based on v7.0-rc2 tag and can be found in git tree here [2]. Merge strategy: ---------------- It is expected due to APIs dependency, the entire patch-set to go via the Qcom tree. All other subsystem maintainers, it will be great if I can get acks for the corresponding subsystem patches. [1] https://github.com/OP-TEE/optee_os/pull/7721 [2] https://git.kernel.org/pub/scm/linux/kernel/git/sumit.garg/linux.git/log/?h… Mukesh Ojha (1): arm64: dts: qcom: kodiak: Add EL2 overlay Sumit Garg (13): firmware: qcom: Add a generic PAS service firmware: qcom_scm: Migrate to generic PAS service firmware: qcom: Add a PAS TEE service remoteproc: qcom_q6v5_pas: Switch over to generic PAS TZ APIs remoteproc: qcom_q6v5_mss: Switch to generic PAS TZ APIs soc: qcom: mdtloader: Switch to generic PAS TZ APIs remoteproc: qcom_wcnss: Switch to generic PAS TZ APIs remoteproc: qcom: Select QCOM_PAS_TEE service backend drm/msm: Switch to generic PAS TZ APIs media: qcom: Switch to generic PAS TZ APIs net: ipa: Switch to generic PAS TZ APIs wifi: ath12k: Switch to generic PAS TZ APIs firmware: qcom_scm: Remove SCM PAS wrappers arch/arm64/boot/dts/qcom/Makefile | 2 + arch/arm64/boot/dts/qcom/kodiak-el2.dtso | 35 ++ drivers/firmware/qcom/Kconfig | 18 + drivers/firmware/qcom/Makefile | 2 + drivers/firmware/qcom/qcom_pas.c | 295 +++++++++++ drivers/firmware/qcom/qcom_pas.h | 53 ++ drivers/firmware/qcom/qcom_pas_tee.c | 478 ++++++++++++++++++ drivers/firmware/qcom/qcom_scm.c | 304 ++++------- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 +- .../media/platform/qcom/iris/iris_firmware.c | 9 +- drivers/media/platform/qcom/venus/firmware.c | 11 +- drivers/net/ipa/ipa_main.c | 13 +- drivers/net/wireless/ath/ath12k/ahb.c | 8 +- drivers/remoteproc/Kconfig | 1 + drivers/remoteproc/qcom_q6v5_mss.c | 5 +- drivers/remoteproc/qcom_q6v5_pas.c | 51 +- drivers/remoteproc/qcom_wcnss.c | 12 +- drivers/soc/qcom/mdt_loader.c | 12 +- include/linux/firmware/qcom/qcom_pas.h | 41 ++ include/linux/firmware/qcom/qcom_scm.h | 29 -- include/linux/soc/qcom/mdt_loader.h | 6 +- 22 files changed, 1097 insertions(+), 303 deletions(-) create mode 100644 arch/arm64/boot/dts/qcom/kodiak-el2.dtso create mode 100644 drivers/firmware/qcom/qcom_pas.c create mode 100644 drivers/firmware/qcom/qcom_pas.h create mode 100644 drivers/firmware/qcom/qcom_pas_tee.c create mode 100644 include/linux/firmware/qcom/qcom_pas.h -- 2.51.0

1 month, 3 weeks

9
38
0 0

[GIT PULL] TEE shared memory fix for 7.0

by Jens Wiklander

Hello arm-soc maintainers, Please pull this patch removing the refcounting of kernel pages within the TEE shared memory helper. Thanks, Jens The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f: Linux 7.0-rc1 (2026-02-22 13:18:59 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git tags/tee-fix-for-v7.0 for you to fetch changes up to 08d9a4580f71120be3c5b221af32dca00a48ceb0: tee: shm: Remove refcounting of kernel pages (2026-03-03 09:03:04 +0100) ---------------------------------------------------------------- TEE shared memory update for 7.0 Remove refcounting of kernel pages in register_shm_helper() to support slab allocations. ---------------------------------------------------------------- Matthew Wilcox (1): tee: shm: Remove refcounting of kernel pages drivers/tee/tee_shm.c | 27 --------------------------- 1 file changed, 27 deletions(-)

1 month, 4 weeks

5
5
0 0

OP-TEE Contributions Forum, monthly meeting March 24

by Jens Wiklander

Hi, Tomorrow, Tuesday, it's time for another LOC monthly meeting. For time and connection details, see the calendar at https://www.trustedfirmware.org/meetings/ - OP-TEE version 4.10.0 to be released April 17 - Marco Felsch is working on initial Kconfig support: https://github.com/OP-TEE/optee_os/pull/7714 + How to configure? Vanilla "make defconfig" and friends? Should we support passing "CFG_XXX=y" on the command line for convenience? In a one-step or two-step operation? + Integration with the build git: Do we need a way to ensure the active configuration matches what the build git expects? + Is it possible to make a seamless transition (with no visible external changes) when building OP-TEE? If so, do we want it? Any other topics? Cheers, Jens

2 months

1
0
0 0

[PATCH v21 0/6] Introduction of a remoteproc tee to load signed firmware

by Arnaud Pouliquen

Main updates from version V20[4]: -------------------------------- To address Rob’s concern on v20concerning resource declaration under the tee node, the device tree is now structured as follows,replacing the child-parent hierarchy with a phandle: firmware { tee_rproc: optee-rproc { compatible = "80a4c275-0a47-4905-8285-1486a9771a08"; }; }; m4: m4@0 { compatible = "st,stm32mp1-m4-tee"; reg = <0 0>; mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>; mbox-names = "vq0", "vq1", "shutdown"; memory-region = <&vdev0vring0>, <&m_ipc_shm>, <&mcuram2>, <&vdev0vring1>, <&vdev0buffer>, <&retram>; interrupt-parent = <&exti>; interrupts = <68 1>; rproc-tee-phandle = <&tee_rproc 0>; st,auto-boot; wakeup-source; status = "okay"; }; As a consequence, this version: - Updates the device tree and bindings to: - Change the compatible property from "rproc-service-80a4c275-0a47-4905-8285-1486a9771a08" to "80a4c275-0a47-4905-8285-1486a9771a08". - Use the rproc-tee-phandle to avoid the parent-child hierarchy. - Updates stm32_rproc_tee.c and remoteproc_tee.c to adapt to the new bindings. - Updates remoteproc_tee.c to compute the device tree compatible string from the TEE UUID. Main updates from version V19[3]: -------------------------------- The devicetree is now structured as follows: firmware { optee { compatible = "linaro,optee-tz"; method = "smc"; #address-cells = <1>; #size-cells = <0>; rproc-service@0 { compatible = "rproc-service-80a4c275-0a47-4905-8285-1486a9771a08"; reg = <0>; #address-cells = <1>; #size-cells = <0>; status = "okay"; m4: m4@0 { compatible = "st,stm32mp15-m4-tee"; reg = <0>; mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>; mbox-names = "vq0", "vq1", "shutdown"; memory-region = <&vdev0vring0>, <&m_ipc_shm>, <&mcuram2>, <&vdev0vring1>, <&vdev0buffer>, <&retram>; interrupt-parent = <&exti>; interrupts = <68 1>; status = "okay"; }; }; }; }; As a consequence, this version: - Introduces a new stm32_rproc_tee.c remoteproc driver. Instead of further complicating the existing stm32_rproc.c driver, a dedicated TEE-based driver is added. Both drivers are intended to also support the STM32MP2x Cortex-M33 remote processor in a next step. - Reworks the bindings: - Drop the st,stm32-rproc.yaml updates that were introduced in previous revisions. - Add remoteproc-tee.yaml for the "rproc-service-80a4c275-0a47-4905-8285-1486a9771a08" compatible. - Add st,stm32-rproc-tee.yaml for the "st,stm32mp15-m4-tee" compatible. - Reworks the probing sequence: The m4@0 device is now probed by the remoteproc-tee driver, which itself is instantiated by the TEE (OP-TEE) bus. Main updates from version V18[2]: -------------------------------- - rework documentation for the release_fw ops - rework function documentation in remoteproc_tee.c - replace spinlock by mutex and generalize usage in remoteproc_tee.c Main updates from version V17[1]: -------------------------------- - Fix: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing More details are available in each patch commit message. [1] https://lore.kernel.org/linux-remoteproc/20250613091650.2337411-1-arnaud.po… [2] https://lore.kernel.org/linux-remoteproc/20250616075530.4106090-1-arnaud.po… [3] https://lore.kernel.org/linux-devicetree/20250625094028.758016-1-arnaud.pou… [3] https://lore.kernel.org/linux-remoteproc/20251217153917.3998544-1-arnaud.po… Tested-on: --------- commit 1f318b96cc84 ("Linux 7.0-rc3") Description of the feature: -------------------------- This series proposes the implementation of a remoteproc tee driver to communicate with a TEE trusted application responsible for authenticating and loading the remoteproc firmware image in an Arm secure context. 1) Principle: The remoteproc tee driver provides services to communicate with the OP-TEE trusted application running on the Trusted Execution Context (TEE). The trusted application in TEE manages the remote processor lifecycle: - authenticating and loading firmware images, - isolating and securing the remote processor memories, - supporting multi-firmware (e.g., TF-M + Zephyr on a Cortex-M33), - managing the start and stop of the firmware by the TEE. 2) Format of the signed image: Refer to: https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/src/remoteproc… 3) OP-TEE trusted application API: Refer to: https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/include/ta_rem… 4) OP-TEE signature script Refer to: https://github.com/OP-TEE/optee_os/blob/master/scripts/sign_rproc_fw.py Example of usage: sign_rproc_fw.py --in <fw1.elf> --in <fw2.elf> --out <signed_fw.sign> --key ${OP-TEE_PATH}/keys/default.pem 5) Impact on User space Application No sysfs impact. The user only needs to provide the signed firmware image instead of the ELF image. For more information about the implementation, a presentation is available here (note that the format of the signed image has evolved between the presentation and the integration in OP-TEE). https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds Arnaud Pouliquen (6): dt-bindings: firmware: Add TEE remoteproc service binding dt-bindings: remoteproc: Add STM32 TEE-controlled rproc binding remoteproc: core: Introduce rproc_pa_to_va helper remoteproc: Introduce optional release_fw operation remoteproc: Add TEE support remoteproc: stm32: Add TEE-controlled STM32 driver .../bindings/remoteproc/remoteproc-tee.yaml | 36 + .../remoteproc/st,stm32-rproc-tee.yaml | 108 +++ drivers/remoteproc/Kconfig | 10 + drivers/remoteproc/Makefile | 3 +- drivers/remoteproc/remoteproc_core.c | 52 ++ drivers/remoteproc/remoteproc_internal.h | 6 + drivers/remoteproc/remoteproc_tee.c | 810 ++++++++++++++++++ drivers/remoteproc/stm32_rproc_tee.c | 537 ++++++++++++ include/linux/remoteproc.h | 6 + include/linux/remoteproc_tee.h | 91 ++ 10 files changed, 1658 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-tee.yaml create mode 100644 Documentation/devicetree/bindings/remoteproc/st,stm32-rproc-tee.yaml create mode 100644 drivers/remoteproc/remoteproc_tee.c create mode 100644 drivers/remoteproc/stm32_rproc_tee.c create mode 100644 include/linux/remoteproc_tee.h base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681 -- 2.43.0

2 months

3
12
0 0

[PATCH] optee: Check return value of tee_shm_get_va()

by Chen Ni

The function tee_shm_get_va() can return an error pointer if the shared memory is not properly mapped or if the offset is invalid. Without this check, passing the error pointer to subsequent memory operations could lead to a kernel panic. Add a check for IS_ERR() on the return value of tee_shm_get_va(). Fixes: f0c8431568ee ("optee: probe RPMB device using RPMB subsystem") Signed-off-by: Chen Ni <nichen(a)iscas.ac.cn> --- drivers/tee/optee/rpc.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index b0ed4cb49452..32f7742c094c 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -393,6 +393,11 @@ static void handle_rpc_func_rpmb_frames(struct tee_context *ctx, params[0].u.memref.shm_offs); p1 = tee_shm_get_va(params[1].u.memref.shm, params[1].u.memref.shm_offs); + if (IS_ERR(p0) || IS_ERR(p1)) { + arg->ret = TEEC_ERROR_BAD_PARAMETERS; + goto out; + } + if (rpmb_route_frames(rdev, p0, params[0].u.memref.size, p1, params[1].u.memref.size)) { arg->ret = TEEC_ERROR_BAD_PARAMETERS; -- 2.25.1

2 months, 1 week

4
4
0 0

[PATCH] tee: clean up tee_core.h kernel-doc

by Randy Dunlap

Use the correct struct member name and function parameter name in kernel-doc comments. Move a macro that was between a struct's documentation and its declaration. These eliminate the following kernel-doc warnings: Warning: include/linux/tee_core.h:73 struct member 'c_no_users' not described in 'tee_device' Warning: include/linux/tee_core.h:132 #define TEE_DESC_PRIVILEGED 0x1; error: Cannot parse struct or union! Warning: include/linux/tee_core.h:257 function parameter 'connection_data' not described in 'tee_session_calc_client_uuid' Warning: include/linux/tee_core.h:320 function parameter 'teedev' not described in 'tee_get_drvdata' Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> --- Cc: Jens Wiklander <jens.wiklander(a)linaro.org> Cc: Sumit Garg <sumit.garg(a)kernel.org> Cc: op-tee(a)lists.trustedfirmware.org include/linux/tee_core.h | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) --- linux-next-20260309.orig/include/linux/tee_core.h +++ linux-next-20260309/include/linux/tee_core.h @@ -50,7 +50,7 @@ enum tee_dma_heap_id { * @dev: embedded basic device structure * @cdev: embedded cdev * @num_users: number of active users of this device - * @c_no_user: completion used when unregistering the device + * @c_no_users: completion used when unregistering the device * @mutex: mutex protecting @num_users and @idr * @idr: register of user space shared memory objects allocated or * registered on this device @@ -132,6 +132,7 @@ struct tee_driver_ops { /* Size for TEE revision string buffer used by get_tee_revision(). */ #define TEE_REVISION_STR_SIZE 128 +#define TEE_DESC_PRIVILEGED 0x1 /** * struct tee_desc - Describes the TEE driver to the subsystem * @name: name of driver @@ -139,7 +140,6 @@ struct tee_driver_ops { * @owner: module providing the driver * @flags: Extra properties of driver, defined by TEE_DESC_* below */ -#define TEE_DESC_PRIVILEGED 0x1 struct tee_desc { const char *name; const struct tee_driver_ops *ops; @@ -187,7 +187,7 @@ struct tee_protmem_pool_ops { * Allocates a new struct tee_device instance. The device is * removed by tee_device_unregister(). * - * @returns a pointer to a 'struct tee_device' or an ERR_PTR on failure + * @returns: a pointer to a 'struct tee_device' or an ERR_PTR on failure */ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc, struct device *dev, @@ -201,7 +201,7 @@ struct tee_device *tee_device_alloc(cons * tee_device_unregister() need to be called to remove the @teedev if * this function fails. * - * @returns < 0 on failure + * @returns: < 0 on failure */ int tee_device_register(struct tee_device *teedev); @@ -254,14 +254,14 @@ void tee_device_set_dev_groups(struct te * tee_session_calc_client_uuid() - Calculates client UUID for session * @uuid: Resulting UUID * @connection_method: Connection method for session (TEE_IOCTL_LOGIN_*) - * @connectuon_data: Connection data for opening session + * @connection_data: Connection data for opening session * * Based on connection method calculates UUIDv5 based client UUID. * * For group based logins verifies that calling process has specified * credentials. * - * @return < 0 on failure + * @returns: < 0 on failure */ int tee_session_calc_client_uuid(uuid_t *uuid, u32 connection_method, const u8 connection_data[TEE_IOCTL_UUID_LEN]); @@ -295,7 +295,7 @@ struct tee_shm_pool_ops { * @paddr: Physical address of start of pool * @size: Size in bytes of the pool * - * @returns pointer to a 'struct tee_shm_pool' or an ERR_PTR on failure. + * @returns: pointer to a 'struct tee_shm_pool' or an ERR_PTR on failure. */ struct tee_shm_pool *tee_shm_pool_alloc_res_mem(unsigned long vaddr, phys_addr_t paddr, size_t size, @@ -318,14 +318,16 @@ static inline void tee_shm_pool_free(str * @paddr: Physical address of start of pool * @size: Size in bytes of the pool * - * @returns pointer to a 'struct tee_protmem_pool' or an ERR_PTR on failure. + * @returns: pointer to a 'struct tee_protmem_pool' or an ERR_PTR on failure. */ struct tee_protmem_pool *tee_protmem_static_pool_alloc(phys_addr_t paddr, size_t size); /** * tee_get_drvdata() - Return driver_data pointer - * @returns the driver_data pointer supplied to tee_register(). + * @teedev: Pointer to the tee_device + * + * @returns: the driver_data pointer supplied to tee_register(). */ void *tee_get_drvdata(struct tee_device *teedev); @@ -334,7 +336,7 @@ void *tee_get_drvdata(struct tee_device * TEE driver * @ctx: The TEE context for shared memory allocation * @size: Shared memory allocation size - * @returns a pointer to 'struct tee_shm' on success or an ERR_PTR on failure + * @returns: a pointer to 'struct tee_shm' on success or an ERR_PTR on failure */ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size); @@ -354,7 +356,7 @@ void tee_dyn_shm_free_helper(struct tee_ /** * tee_shm_is_dynamic() - Check if shared memory object is of the dynamic kind * @shm: Shared memory handle - * @returns true if object is dynamic shared memory + * @returns: true if object is dynamic shared memory */ static inline bool tee_shm_is_dynamic(struct tee_shm *shm) { @@ -370,7 +372,7 @@ void tee_shm_put(struct tee_shm *shm); /** * tee_shm_get_id() - Get id of a shared memory object * @shm: Shared memory handle - * @returns id + * @returns: id */ static inline int tee_shm_get_id(struct tee_shm *shm) { @@ -382,7 +384,7 @@ static inline int tee_shm_get_id(struct * count * @ctx: Context owning the shared memory * @id: Id of shared memory object - * @returns a pointer to 'struct tee_shm' on success or an ERR_PTR on failure + * @returns: a pointer to 'struct tee_shm' on success or an ERR_PTR on failure */ struct tee_shm *tee_shm_get_from_id(struct tee_context *ctx, int id); @@ -402,7 +404,7 @@ static inline bool tee_param_is_memref(s * teedev_open() - Open a struct tee_device * @teedev: Device to open * - * @return a pointer to struct tee_context on success or an ERR_PTR on failure. + * @returns: pointer to struct tee_context on success or an ERR_PTR on failure. */ struct tee_context *teedev_open(struct tee_device *teedev);

2 months, 1 week

3
2
0 0

[PATCH v4] arm64: defconfig: Enable QCOMTEE module for QTEE-enabled Qualcomm SoCs

by Harshal Dev

All Qualcomm SoCs starting from SM8650 provide access to the Qualcomm Trusted Execution Environment (QTEE) through the SMCInvoke interface, implemented by the QCOMTEE driver. QTEE runs in the Secure World domain on ARM64 CPUs and exposes secure services to Linux running in the Normal World domain. This change enables the QCOMTEE driver as a module to support communication with QTEE. QCOMTEE has been tested on a Qualcomm RB3Gen2 board by loading and executing a Trusted Application via tests hosted at github.com/qualcomm/minkipc. Signed-off-by: Harshal Dev <harshal.dev(a)oss.qualcomm.com> --- Changes in v4: - Updated the commit message as per discussion to clarify the following: - Why we are enabling QCOMTEE for all arm64 boards. - From which Qualcomm SoC onwards the driver is applicable. - What functionality the driver provides. - How the functionality has been tested and on which board. - Link to v3: https://lore.kernel.org/r/20251208-qcom_qcomtee_defconfig-v3-1-b50dcf8ab45e… Changes in v3: - Updated the commit message to reflect the supported Qualcomm platforms. - Link to v2: https://lore.kernel.org/r/20251205-qcom_qcomtee_defconfig-v2-1-c92560b0346e… Changes in v2: - Updated CONFIG_QCOMTEE flag to 'm' since QCOMTEE can be built as a module. - Link to v1: https://lore.kernel.org/r/20251202-qcom_qcomtee_defconfig-v1-1-11bfe40a8ea4… --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index cdb7d69e3b24..e952d24bef77 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1789,6 +1789,7 @@ CONFIG_FPGA_MGR_ZYNQMP_FPGA=m CONFIG_FPGA_MGR_VERSAL_FPGA=m CONFIG_TEE=y CONFIG_OPTEE=y +CONFIG_QCOMTEE=m CONFIG_MUX_GPIO=m CONFIG_MUX_MMIO=y CONFIG_SLIMBUS=m --- base-commit: 47b7b5e32bb7264b51b89186043e1ada4090b558 change-id: 20251202-qcom_qcomtee_defconfig-8dc0fed1411b Best regards, -- Harshal Dev <harshal.dev(a)oss.qualcomm.com>

2 months, 1 week

6
12
0 0

[RFT PATCH v2] tee: shm: Remove refcounting of kernel pages

by Sumit Garg

From: Matthew Wilcox <willy(a)infradead.org> Earlier TEE subsystem assumed to refcount all the memory pages to be shared with TEE implementation to be refcounted. However, the slab allocations within the kernel don't allow refcounting kernel pages. It is rather better to trust the kernel clients to not free pages while being shared with TEE implementation. Hence, remove refcounting of kernel pages from register_shm_helper() API. Fixes: b9c0e49abfca ("mm: decline to manipulate the refcount on a slab page") Reported-by: Marco Felsch <m.felsch(a)pengutronix.de> Reported-by: Sven Püschel <s.pueschel(a)pengutronix.de> Signed-off-by: Matthew Wilcox <willy(a)infradead.org> Co-developed-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> Signed-off-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> --- Changes in v2: - Attribute Matthew as the author of this patch. - Fix check for user pages. drivers/tee/tee_shm.c | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 4a47de4bb2e5..898707ca21a8 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -23,29 +23,11 @@ struct tee_shm_dma_mem { struct page *page; }; -static void shm_put_kernel_pages(struct page **pages, size_t page_count) -{ - size_t n; - - for (n = 0; n < page_count; n++) - put_page(pages[n]); -} - -static void shm_get_kernel_pages(struct page **pages, size_t page_count) -{ - size_t n; - - for (n = 0; n < page_count; n++) - get_page(pages[n]); -} - static void release_registered_pages(struct tee_shm *shm) { if (shm->pages) { if (shm->flags & TEE_SHM_USER_MAPPED) unpin_user_pages(shm->pages, shm->num_pages); - else - shm_put_kernel_pages(shm->pages, shm->num_pages); kfree(shm->pages); } @@ -477,13 +459,6 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags, goto err_put_shm_pages; } - /* - * iov_iter_extract_kvec_pages does not get reference on the pages, - * get a reference on them. - */ - if (iov_iter_is_kvec(iter)) - shm_get_kernel_pages(shm->pages, num_pages); - shm->offset = off; shm->size = len; shm->num_pages = num_pages; @@ -499,8 +474,6 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags, err_put_shm_pages: if (!iov_iter_is_kvec(iter)) unpin_user_pages(shm->pages, shm->num_pages); - else - shm_put_kernel_pages(shm->pages, shm->num_pages); err_free_shm_pages: kfree(shm->pages); err_free_shm: -- 2.51.0

2 months, 1 week

3
2
0 0

[PATCH 0/3] OP-TEE/OP-TEE drivers: simplify context matches

by Rouven Czerwinski via B4 Relay

Both the OP-TEE core and some OP-TEE drivers use an if/else expression to check a boolean which can instead be returned directly. Implement this change. --- Rouven Czerwinski (3): optee: simplify OP-TEE context match hwrng: optee - simplify OP-TEE context match rtc: optee: simplify OP-TEE context match drivers/char/hw_random/optee-rng.c | 5 +---- drivers/rtc/rtc-optee.c | 5 +---- drivers/tee/optee/device.c | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-) --- base-commit: 63804fed149a6750ffd28610c5c1c98cce6bd377 change-id: 20260126-optee-simplify-context-match-d5b3467bfacc Best regards, -- Rouven Czerwinski <rouven.czerwinski(a)linaro.org>

2 months, 2 weeks

6
8
0 0

Re: [PATCH v5] tee: optee: prevent use-after-free when the client exits before the supplicant

by Yuanjia Yeh

Hi all, We can reproduce the reported use-after-free issue on our platform via an overnight reboot test The verifications are listed as the following - With v2 + Michael Wu's patch: Issue resolved. - With v4: Race condition observation and already report to Amir - With v5: Race fixed. Stable in our tests, including: xtest + reboot loop (300 cycles) continuous reboot test (1000 cycles) No regressions observed with v5. If there are no other concerns, we recommend adopting v5. If needed, please add the following tag: Tested-by: Ox Yeh <ox.yeh(a)mediatek.com> Yuanjia Yeh

2 months, 2 weeks

3
3
0 0

OP-TEE Contributions Forum, monthly meeting February 24

by Jens Wiklander

Hi, Tomorrow, Tuesday, it's time for another LOC monthly meeting. For time and connection details, see the calendar at https://www.trustedfirmware.org/meetings/ - Rebased linaro-swg/kernel optee branch on v6.18 Any other topics? Cheers, Jens

2 months, 3 weeks

1
0
0 0

[RFT PATCH] tee: shm: Remove refcounting of kernel pages

by Sumit Garg

From: Sumit Garg <sumit.garg(a)oss.qualcomm.com> Earlier TEE subsystem assumed to refcount all the memory pages to be shared with TEE implementation to be refcounted. However, the slab allocations within the kernel don't allow refcounting kernel pages. It is rather better to trust the kernel clients to not free pages while being shared with TEE implementation. Hence, remove refcounting of kernel pages from register_shm_helper() API. Fixes: b9c0e49abfca ("mm: decline to manipulate the refcount on a slab page") Reported-by: Marco Felsch <m.felsch(a)pengutronix.de> Reported-by: Sven Püschel <s.pueschel(a)pengutronix.de> Suggested-by: Matthew Wilcox <willy(a)infradead.org> Signed-off-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> --- drivers/tee/tee_shm.c | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 4a47de4bb2e5..54e2ba3afb25 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -23,29 +23,11 @@ struct tee_shm_dma_mem { struct page *page; }; -static void shm_put_kernel_pages(struct page **pages, size_t page_count) -{ - size_t n; - - for (n = 0; n < page_count; n++) - put_page(pages[n]); -} - -static void shm_get_kernel_pages(struct page **pages, size_t page_count) -{ - size_t n; - - for (n = 0; n < page_count; n++) - get_page(pages[n]); -} - static void release_registered_pages(struct tee_shm *shm) { if (shm->pages) { if (shm->flags & TEE_SHM_USER_MAPPED) unpin_user_pages(shm->pages, shm->num_pages); - else - shm_put_kernel_pages(shm->pages, shm->num_pages); kfree(shm->pages); } @@ -477,13 +459,6 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags, goto err_put_shm_pages; } - /* - * iov_iter_extract_kvec_pages does not get reference on the pages, - * get a reference on them. - */ - if (iov_iter_is_kvec(iter)) - shm_get_kernel_pages(shm->pages, num_pages); - shm->offset = off; shm->size = len; shm->num_pages = num_pages; @@ -497,10 +472,8 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags, return shm; err_put_shm_pages: - if (!iov_iter_is_kvec(iter)) + if (iter_is_uvec(iter)) unpin_user_pages(shm->pages, shm->num_pages); - else - shm_put_kernel_pages(shm->pages, shm->num_pages); err_free_shm_pages: kfree(shm->pages); err_free_shm: -- 2.51.0

2 months, 4 weeks

4
6
0 0

[PATCH v20 0/6] Introduction of a remoteproc tee to load signed firmware

by Arnaud Pouliquen

Main updates from version V19[3]: -------------------------------- The devicetree is now structured as follows: firmware { optee { compatible = "linaro,optee-tz"; method = "smc"; #address-cells = <1>; #size-cells = <0>; rproc-service@0 { compatible = "rproc-service-80a4c275-0a47-4905-8285-1486a9771a08"; reg = <0>; #address-cells = <1>; #size-cells = <0>; status = "okay"; m4: m4@0 { compatible = "st,stm32mp15-m4-tee"; reg = <0>; mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>; mbox-names = "vq0", "vq1", "shutdown"; memory-region = <&vdev0vring0>, <&m_ipc_shm>, <&mcuram2>, <&vdev0vring1>, <&vdev0buffer>, <&retram>; interrupt-parent = <&exti>; interrupts = <68 1>; status = "okay"; }; }; }; }; As a consequence, this version: - Introduces a new stm32_rproc_tee.c remoteproc driver. Instead of further complicating the existing stm32_rproc.c driver, a dedicated TEE-based driver is added. Both drivers are intended to also support the STM32MP2x Cortex-M33 remote processor in a next step. - Reworks the bindings: - Drop the st,stm32-rproc.yaml updates that were introduced in previous revisions. - Add remoteproc-tee.yaml for the "rproc-service-80a4c275-0a47-4905-8285-1486a9771a08" compatible. - Add st,stm32-rproc-tee.yaml for the "st,stm32mp15-m4-tee" compatible. - Reworks the probing sequence: The m4@0 device is now probed by the remoteproc-tee driver, which itself is instantiated by the TEE (OP-TEE) bus. Main updates from version V18[2]: -------------------------------- - rework documentation for the release_fw ops - rework function documentation in remoteproc_tee.c - replace spinlock by mutex and generalize usage in remoteproc_tee.c Main updates from version V17[1]: -------------------------------- - Fix: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing More details are available in each patch commit message. [1] https://lore.kernel.org/linux-remoteproc/20250613091650.2337411-1-arnaud.po… [2] https://lore.kernel.org/linux-remoteproc/20250616075530.4106090-1-arnaud.po… [3] https://lore.kernel.org/linux-devicetree/20250625094028.758016-1-arnaud.pou… Tested-on: --------- commit 7d0a66e4bb90 ("Linux 6.18") Description of the feature: -------------------------- This series proposes the implementation of a remoteproc tee driver to communicate with a TEE trusted application responsible for authenticating and loading the remoteproc firmware image in an Arm secure context. 1) Principle: The remoteproc tee driver provides services to communicate with the OP-TEE trusted application running on the Trusted Execution Context (TEE). The trusted application in TEE manages the remote processor lifecycle: - authenticating and loading firmware images, - isolating and securing the remote processor memories, - supporting multi-firmware (e.g., TF-M + Zephyr on a Cortex-M33), - managing the start and stop of the firmware by the TEE. 2) Format of the signed image: Refer to: https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/src/remoteproc… 3) OP-TEE trusted application API: Refer to: https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/include/ta_rem… 4) OP-TEE signature script Refer to: https://github.com/OP-TEE/optee_os/blob/master/scripts/sign_rproc_fw.py Example of usage: sign_rproc_fw.py --in <fw1.elf> --in <fw2.elf> --out <signed_fw.sign> --key ${OP-TEE_PATH}/keys/default.pem 5) Impact on User space Application No sysfs impact. The user only needs to provide the signed firmware image instead of the ELF image. For more information about the implementation, a presentation is available here (note that the format of the signed image has evolved between the presentation and the integration in OP-TEE). https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds Arnaud Pouliquen (6): dt-bindings: firmware: Add TEE remoteproc service binding dt-bindings: remoteproc: Add STM32 TEE-controlled rproc binding remoteproc: core: Introduce rproc_pa_to_va helper remoteproc: Introduce optional release_fw operation remoteproc: Add TEE support remoteproc: stm32: Add TEE-controlled STM32 driver .../arm/firmware/linaro,optee-tz.yaml | 6 + .../bindings/remoteproc/remoteproc-tee.yaml | 47 ++ .../remoteproc/st,stm32-rproc-tee.yaml | 100 +++ drivers/remoteproc/Kconfig | 10 + drivers/remoteproc/Makefile | 3 +- drivers/remoteproc/remoteproc_core.c | 52 ++ drivers/remoteproc/remoteproc_internal.h | 6 + drivers/remoteproc/remoteproc_tee.c | 771 ++++++++++++++++++ drivers/remoteproc/stm32_rproc_tee.c | 526 ++++++++++++ include/linux/remoteproc.h | 6 + include/linux/remoteproc_tee.h | 89 ++ 11 files changed, 1615 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-tee.yaml create mode 100644 Documentation/devicetree/bindings/remoteproc/st,stm32-rproc-tee.yaml create mode 100644 drivers/remoteproc/remoteproc_tee.c create mode 100644 drivers/remoteproc/stm32_rproc_tee.c create mode 100644 include/linux/remoteproc_tee.h base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449 -- 2.43.0

3 months

4
17
0 0

[PATCH v5] tee: optee: prevent use-after-free when the client exits before the supplicant

by Amirreza Zarrabi

Commit 70b0d6b0a199 ("tee: optee: Fix supplicant wait loop") made the client wait as killable so it can be interrupted during shutdown or after a supplicant crash. This changes the original lifetime expectations: the client task can now terminate while the supplicant is still processing its request. If the client exits first it removes the request from its queue and kfree()s it, while the request ID remains in supp->idr. A subsequent lookup on the supplicant path then dereferences freed memory, leading to a use-after-free. Serialise access to the request with supp->mutex: * Hold supp->mutex in optee_supp_recv() and optee_supp_send() while looking up and touching the request. * Let optee_supp_thrd_req() notice that the client has terminated and signal optee_supp_send() accordingly. With these changes the request cannot be freed while the supplicant still has a reference, eliminating the race. Fixes: 70b0d6b0a199 ("tee: optee: Fix supplicant wait loop") Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com> --- Changes in v5: - Move idr_alloc() inside the supp->mutex, reported by Yuanjia Yeh. - Link to v4: https://lore.kernel.org/r/20260209-fix-use-after-free-v4-1-7c4c4b02368f@oss… Changes in v4: - Pre-allocate request ID when allocating the request. - Cleanup the loop in optee_supp_recv(). - Update the return value for revoked request from -ENOENT to -EBADF. - Link to v3: https://lore.kernel.org/r/20260128-fix-use-after-free-v3-1-b0786670d927@oss… Changes in v3: - Introduce processed flag instead of -1 for req->id. - Update optee_supp_release() as reported by Michael Wu. - Use mutex instead of guard. - Link to v2: https://lore.kernel.org/r/20250617-fix-use-after-free-v2-1-1fbfafec5917@oss… Changes in v2: - Replace the static variable with a sentinel value. - Fix the issue with returning the popped request to the supplicant. - Link to v1: https://lore.kernel.org/r/20250605-fix-use-after-free-v1-1-a70d23bff248@oss… --- drivers/tee/optee/supp.c | 107 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 33 deletions(-) diff --git a/drivers/tee/optee/supp.c b/drivers/tee/optee/supp.c index d0f397c90242..2386bbd38ce7 100644 --- a/drivers/tee/optee/supp.c +++ b/drivers/tee/optee/supp.c @@ -10,7 +10,11 @@ struct optee_supp_req { struct list_head link; + int id; + bool in_queue; + bool processed; + u32 func; u32 ret; size_t num_params; @@ -19,6 +23,9 @@ struct optee_supp_req { struct completion c; }; +/* It is temporary request used for revoked pending request in supp->idr. */ +#define INVALID_REQ_PTR ((struct optee_supp_req *)ERR_PTR(-EBADF)) + void optee_supp_init(struct optee_supp *supp) { memset(supp, 0, sizeof(*supp)); @@ -39,21 +46,23 @@ void optee_supp_release(struct optee_supp *supp) { int id; struct optee_supp_req *req; - struct optee_supp_req *req_tmp; mutex_lock(&supp->mutex); - /* Abort all request retrieved by supplicant */ + /* Abort all request */ idr_for_each_entry(&supp->idr, req, id) { idr_remove(&supp->idr, id); - req->ret = TEEC_ERROR_COMMUNICATION; - complete(&req->c); - } + /* Skip if request was already marked invalid */ + if (IS_ERR(req)) + continue; - /* Abort all queued requests */ - list_for_each_entry_safe(req, req_tmp, &supp->reqs, link) { - list_del(&req->link); - req->in_queue = false; + /* For queued requests where supplicant has not seen it */ + if (req->in_queue) { + list_del(&req->link); + req->in_queue = false; + } + + req->processed = true; req->ret = TEEC_ERROR_COMMUNICATION; complete(&req->c); } @@ -100,8 +109,16 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, /* Insert the request in the request list */ mutex_lock(&supp->mutex); + req->id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL); + if (req->id < 0) { + mutex_unlock(&supp->mutex); + kfree(req); + return TEEC_ERROR_OUT_OF_MEMORY; + } + list_add_tail(&req->link, &supp->reqs); req->in_queue = true; + req->processed = false; mutex_unlock(&supp->mutex); /* Tell an eventual waiter there's a new request */ @@ -117,21 +134,43 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, if (wait_for_completion_killable(&req->c)) { mutex_lock(&supp->mutex); if (req->in_queue) { + /* Supplicant has not seen this request yet. */ + idr_remove(&supp->idr, req->id); list_del(&req->link); req->in_queue = false; + + ret = TEEC_ERROR_COMMUNICATION; + } else if (req->processed) { + /* + * Supplicant has processed this request. Ignore the + * kill signal for now and submit the result. req is not + * in supp->reqs (removed by supp_pop_entry()) nor in + * supp->idr (removed by supp_pop_req()). + */ + ret = req->ret; + } else { + /* + * Supplicant is in the middle of processing this + * request. Replace req with INVALID_REQ_PTR so that + * the ID remains busy, causing optee_supp_send() to + * fail on the next call to supp_pop_req() with this ID. + */ + idr_replace(&supp->idr, INVALID_REQ_PTR, req->id); + ret = TEEC_ERROR_COMMUNICATION; } + mutex_unlock(&supp->mutex); - req->ret = TEEC_ERROR_COMMUNICATION; + } else { + ret = req->ret; } - ret = req->ret; kfree(req); return ret; } static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, - int num_params, int *id) + int num_params) { struct optee_supp_req *req; @@ -153,10 +192,6 @@ static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, return ERR_PTR(-EINVAL); } - *id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL); - if (*id < 0) - return ERR_PTR(-ENOMEM); - list_del(&req->link); req->in_queue = false; @@ -214,7 +249,6 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, struct optee *optee = tee_get_drvdata(teedev); struct optee_supp *supp = &optee->supp; struct optee_supp_req *req = NULL; - int id; size_t num_meta; int rc; @@ -224,15 +258,11 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, while (true) { mutex_lock(&supp->mutex); - req = supp_pop_entry(supp, *num_params - num_meta, &id); + req = supp_pop_entry(supp, *num_params - num_meta); + if (req) + break; /* Keep mutex held. */ mutex_unlock(&supp->mutex); - if (req) { - if (IS_ERR(req)) - return PTR_ERR(req); - break; - } - /* * If we didn't get a request we'll block in * wait_for_completion() to avoid needless spinning. @@ -245,6 +275,13 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, return -ERESTARTSYS; } + /* supp->mutex held and req != NULL. */ + + if (IS_ERR(req)) { + mutex_unlock(&supp->mutex); + return PTR_ERR(req); + } + if (num_meta) { /* * tee-supplicant support meta parameters -> requsts can be @@ -252,13 +289,11 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, */ param->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT | TEE_IOCTL_PARAM_ATTR_META; - param->u.value.a = id; + param->u.value.a = req->id; param->u.value.b = 0; param->u.value.c = 0; } else { - mutex_lock(&supp->mutex); - supp->req_id = id; - mutex_unlock(&supp->mutex); + supp->req_id = req->id; } *func = req->func; @@ -266,6 +301,7 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, memcpy(param + num_meta, req->param, sizeof(struct tee_param) * req->num_params); + mutex_unlock(&supp->mutex); return 0; } @@ -297,12 +333,17 @@ static struct optee_supp_req *supp_pop_req(struct optee_supp *supp, if (!req) return ERR_PTR(-ENOENT); + /* optee_supp_thrd_req() already returned to optee. */ + if (IS_ERR(req)) + goto failed_req; + if ((num_params - nm) != req->num_params) return ERR_PTR(-EINVAL); + *num_meta = nm; +failed_req: idr_remove(&supp->idr, id); supp->req_id = -1; - *num_meta = nm; return req; } @@ -328,10 +369,9 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params, mutex_lock(&supp->mutex); req = supp_pop_req(supp, num_params, param, &num_meta); - mutex_unlock(&supp->mutex); - if (IS_ERR(req)) { - /* Something is wrong, let supplicant restart. */ + mutex_unlock(&supp->mutex); + /* Something is wrong, let supplicant handel it. */ return PTR_ERR(req); } @@ -355,9 +395,10 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params, } } req->ret = ret; - + req->processed = true; /* Let the requesting thread continue */ complete(&req->c); + mutex_unlock(&supp->mutex); return 0; } --- base-commit: 3f24e4edcd1b8981c6b448ea2680726dedd87279 change-id: 20250604-fix-use-after-free-8ff1b5d5d774 Best regards, -- Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com>

3 months

1
0
0 0

[PATCH v2] tee: shm: fix slab page refcounting

by Marco Felsch

Skip manipulating the refcount in case of slab pages according commit b9c0e49abfca ("mm: decline to manipulate the refcount on a slab page"). Fixes: b9c0e49abfca ("mm: decline to manipulate the refcount on a slab page") Signed-off-by: Marco Felsch <m.felsch(a)pengutronix.de> --- v2: - Make use of page variable v1: - https://lore.kernel.org/all/20250325195021.3589797-1-m.felsch@pengutronix.d… drivers/tee/tee_shm.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index daf6e5cfd59a..35f0ac359b12 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -19,16 +19,24 @@ static void shm_put_kernel_pages(struct page **pages, size_t page_count) { size_t n; - for (n = 0; n < page_count; n++) - put_page(pages[n]); + for (n = 0; n < page_count; n++) { + struct page *page = pages[n]; + + if (!PageSlab(page)) + put_page(page); + } } static void shm_get_kernel_pages(struct page **pages, size_t page_count) { size_t n; - for (n = 0; n < page_count; n++) - get_page(pages[n]); + for (n = 0; n < page_count; n++) { + struct page *page = pages[n]; + + if (!PageSlab(page)) + get_page(page); + } } static void release_registered_pages(struct tee_shm *shm) -- 2.39.5

3 months

5
17
0 0

[PATCH v4] tee: optee: prevent use-after-free when the client exits before the supplicant

by Amirreza Zarrabi

Commit 70b0d6b0a199 ("tee: optee: Fix supplicant wait loop") made the client wait as killable so it can be interrupted during shutdown or after a supplicant crash. This changes the original lifetime expectations: the client task can now terminate while the supplicant is still processing its request. If the client exits first it removes the request from its queue and kfree()s it, while the request ID remains in supp->idr. A subsequent lookup on the supplicant path then dereferences freed memory, leading to a use-after-free. Serialise access to the request with supp->mutex: * Hold supp->mutex in optee_supp_recv() and optee_supp_send() while looking up and touching the request. * Let optee_supp_thrd_req() notice that the client has terminated and signal optee_supp_send() accordingly. With these changes the request cannot be freed while the supplicant still has a reference, eliminating the race. Fixes: 70b0d6b0a199 ("tee: optee: Fix supplicant wait loop") Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com> --- Changes in v4: - Pre-allocate request ID when allocating the request. - Cleanup the loop in optee_supp_recv(). - Update the return value for revoked request from -ENOENT to -EBADF. - Link to v3: https://lore.kernel.org/r/20260128-fix-use-after-free-v3-1-b0786670d927@oss… Changes in v3: - Introduce processed flag instead of -1 for req->id. - Update optee_supp_release() as reported by Michael Wu. - Use mutex instead of guard. - Link to v2: https://lore.kernel.org/r/20250617-fix-use-after-free-v2-1-1fbfafec5917@oss… Changes in v2: - Replace the static variable with a sentinel value. - Fix the issue with returning the popped request to the supplicant. - Link to v1: https://lore.kernel.org/r/20250605-fix-use-after-free-v1-1-a70d23bff248@oss… --- drivers/tee/optee/supp.c | 106 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 33 deletions(-) diff --git a/drivers/tee/optee/supp.c b/drivers/tee/optee/supp.c index d0f397c90242..c1ae76df7067 100644 --- a/drivers/tee/optee/supp.c +++ b/drivers/tee/optee/supp.c @@ -10,7 +10,11 @@ struct optee_supp_req { struct list_head link; + int id; + bool in_queue; + bool processed; + u32 func; u32 ret; size_t num_params; @@ -19,6 +23,9 @@ struct optee_supp_req { struct completion c; }; +/* It is temporary request used for revoked pending request in supp->idr. */ +#define INVALID_REQ_PTR ((struct optee_supp_req *)ERR_PTR(-EBADF)) + void optee_supp_init(struct optee_supp *supp) { memset(supp, 0, sizeof(*supp)); @@ -39,21 +46,23 @@ void optee_supp_release(struct optee_supp *supp) { int id; struct optee_supp_req *req; - struct optee_supp_req *req_tmp; mutex_lock(&supp->mutex); - /* Abort all request retrieved by supplicant */ + /* Abort all request */ idr_for_each_entry(&supp->idr, req, id) { idr_remove(&supp->idr, id); - req->ret = TEEC_ERROR_COMMUNICATION; - complete(&req->c); - } + /* Skip if request was already marked invalid */ + if (IS_ERR(req)) + continue; - /* Abort all queued requests */ - list_for_each_entry_safe(req, req_tmp, &supp->reqs, link) { - list_del(&req->link); - req->in_queue = false; + /* For queued requests where supplicant has not seen it */ + if (req->in_queue) { + list_del(&req->link); + req->in_queue = false; + } + + req->processed = true; req->ret = TEEC_ERROR_COMMUNICATION; complete(&req->c); } @@ -93,6 +102,12 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, if (!req) return TEEC_ERROR_OUT_OF_MEMORY; + req->id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL); + if (req->id < 0) { + kfree(req); + return TEEC_ERROR_OUT_OF_MEMORY; + } + init_completion(&req->c); req->func = func; req->num_params = num_params; @@ -102,6 +117,7 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, mutex_lock(&supp->mutex); list_add_tail(&req->link, &supp->reqs); req->in_queue = true; + req->processed = false; mutex_unlock(&supp->mutex); /* Tell an eventual waiter there's a new request */ @@ -117,21 +133,43 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, if (wait_for_completion_killable(&req->c)) { mutex_lock(&supp->mutex); if (req->in_queue) { + /* Supplicant has not seen this request yet. */ + idr_remove(&supp->idr, req->id); list_del(&req->link); req->in_queue = false; + + ret = TEEC_ERROR_COMMUNICATION; + } else if (req->processed) { + /* + * Supplicant has processed this request. Ignore the + * kill signal for now and submit the result. req is not + * in supp->reqs (removed by supp_pop_entry()) nor in + * supp->idr (removed by supp_pop_req()). + */ + ret = req->ret; + } else { + /* + * Supplicant is in the middle of processing this + * request. Replace req with INVALID_REQ_PTR so that + * the ID remains busy, causing optee_supp_send() to + * fail on the next call to supp_pop_req() with this ID. + */ + idr_replace(&supp->idr, INVALID_REQ_PTR, req->id); + ret = TEEC_ERROR_COMMUNICATION; } + mutex_unlock(&supp->mutex); - req->ret = TEEC_ERROR_COMMUNICATION; + } else { + ret = req->ret; } - ret = req->ret; kfree(req); return ret; } static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, - int num_params, int *id) + int num_params) { struct optee_supp_req *req; @@ -153,10 +191,6 @@ static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, return ERR_PTR(-EINVAL); } - *id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL); - if (*id < 0) - return ERR_PTR(-ENOMEM); - list_del(&req->link); req->in_queue = false; @@ -214,7 +248,6 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, struct optee *optee = tee_get_drvdata(teedev); struct optee_supp *supp = &optee->supp; struct optee_supp_req *req = NULL; - int id; size_t num_meta; int rc; @@ -224,15 +257,11 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, while (true) { mutex_lock(&supp->mutex); - req = supp_pop_entry(supp, *num_params - num_meta, &id); + req = supp_pop_entry(supp, *num_params - num_meta); + if (req) + break; /* Keep mutex held. */ mutex_unlock(&supp->mutex); - if (req) { - if (IS_ERR(req)) - return PTR_ERR(req); - break; - } - /* * If we didn't get a request we'll block in * wait_for_completion() to avoid needless spinning. @@ -245,6 +274,13 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, return -ERESTARTSYS; } + /* supp->mutex held and req != NULL. */ + + if (IS_ERR(req)) { + mutex_unlock(&supp->mutex); + return PTR_ERR(req); + } + if (num_meta) { /* * tee-supplicant support meta parameters -> requsts can be @@ -252,13 +288,11 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, */ param->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT | TEE_IOCTL_PARAM_ATTR_META; - param->u.value.a = id; + param->u.value.a = req->id; param->u.value.b = 0; param->u.value.c = 0; } else { - mutex_lock(&supp->mutex); - supp->req_id = id; - mutex_unlock(&supp->mutex); + supp->req_id = req->id; } *func = req->func; @@ -266,6 +300,7 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, memcpy(param + num_meta, req->param, sizeof(struct tee_param) * req->num_params); + mutex_unlock(&supp->mutex); return 0; } @@ -297,12 +332,17 @@ static struct optee_supp_req *supp_pop_req(struct optee_supp *supp, if (!req) return ERR_PTR(-ENOENT); + /* optee_supp_thrd_req() already returned to optee. */ + if (IS_ERR(req)) + goto failed_req; + if ((num_params - nm) != req->num_params) return ERR_PTR(-EINVAL); + *num_meta = nm; +failed_req: idr_remove(&supp->idr, id); supp->req_id = -1; - *num_meta = nm; return req; } @@ -328,10 +368,9 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params, mutex_lock(&supp->mutex); req = supp_pop_req(supp, num_params, param, &num_meta); - mutex_unlock(&supp->mutex); - if (IS_ERR(req)) { - /* Something is wrong, let supplicant restart. */ + mutex_unlock(&supp->mutex); + /* Something is wrong, let supplicant handel it. */ return PTR_ERR(req); } @@ -355,9 +394,10 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params, } } req->ret = ret; - + req->processed = true; /* Let the requesting thread continue */ complete(&req->c); + mutex_unlock(&supp->mutex); return 0; } --- base-commit: 3f24e4edcd1b8981c6b448ea2680726dedd87279 change-id: 20250604-fix-use-after-free-8ff1b5d5d774 Best regards, -- Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com>

3 months, 1 week

2
1
0 0

[PATCH v3] tee: optee: prevent use-after-free when the client exits before the supplicant

by Amirreza Zarrabi

Commit 70b0d6b0a199 ("tee: optee: Fix supplicant wait loop") made the client wait as killable so it can be interrupted during shutdown or after a supplicant crash. This changes the original lifetime expectations: the client task can now terminate while the supplicant is still processing its request. If the client exits first it removes the request from its queue and kfree()s it, while the request ID remains in supp->idr. A subsequent lookup on the supplicant path then dereferences freed memory, leading to a use-after-free. Serialise access to the request with supp->mutex: * Hold supp->mutex in optee_supp_recv() and optee_supp_send() while looking up and touching the request. * Let optee_supp_thrd_req() notice that the client has terminated and signal optee_supp_send() accordingly. With these changes the request cannot be freed while the supplicant still has a reference, eliminating the race. Fixes: 70b0d6b0a199 ("tee: optee: Fix supplicant wait loop") Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com> --- Changes in v3: - Introduce processed flag instead of -1 for req->id. - Update optee_supp_release() as reported by Michael Wu. - Use mutex instead of guard. - Link to v2: https://lore.kernel.org/r/20250617-fix-use-after-free-v2-1-1fbfafec5917@oss… Changes in v2: - Replace the static variable with a sentinel value. - Fix the issue with returning the popped request to the supplicant. - Link to v1: https://lore.kernel.org/r/20250605-fix-use-after-free-v1-1-a70d23bff248@oss… --- drivers/tee/optee/supp.c | 122 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 86 insertions(+), 36 deletions(-) diff --git a/drivers/tee/optee/supp.c b/drivers/tee/optee/supp.c index d0f397c90242..0ec66008df19 100644 --- a/drivers/tee/optee/supp.c +++ b/drivers/tee/optee/supp.c @@ -10,7 +10,11 @@ struct optee_supp_req { struct list_head link; + int id; + bool in_queue; + bool processed; + u32 func; u32 ret; size_t num_params; @@ -19,6 +23,9 @@ struct optee_supp_req { struct completion c; }; +/* It is temporary request used for invalid pending request in supp->idr. */ +#define INVALID_REQ_PTR ((struct optee_supp_req *)ERR_PTR(-ENOENT)) + void optee_supp_init(struct optee_supp *supp) { memset(supp, 0, sizeof(*supp)); @@ -46,6 +53,10 @@ void optee_supp_release(struct optee_supp *supp) /* Abort all request retrieved by supplicant */ idr_for_each_entry(&supp->idr, req, id) { idr_remove(&supp->idr, id); + /* Skip if request was already marked invalid */ + if (IS_ERR(req)) + continue; + req->ret = TEEC_ERROR_COMMUNICATION; complete(&req->c); } @@ -102,6 +113,7 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, mutex_lock(&supp->mutex); list_add_tail(&req->link, &supp->reqs); req->in_queue = true; + req->processed = false; mutex_unlock(&supp->mutex); /* Tell an eventual waiter there's a new request */ @@ -117,21 +129,40 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, if (wait_for_completion_killable(&req->c)) { mutex_lock(&supp->mutex); if (req->in_queue) { + /* Supplicant has not seen this request yet. */ list_del(&req->link); req->in_queue = false; + + ret = TEEC_ERROR_COMMUNICATION; + } else if (req->processed) { + /* + * Supplicant has processed this request. Ignore the + * kill signal for now and submit the result. + */ + ret = req->ret; + } else { + /* + * Supplicant is in the middle of processing this + * request. Replace req with INVALID_REQ_PTR so that + * the ID remains busy, causing optee_supp_send() to + * fail on the next call to supp_pop_req() with this ID. + */ + idr_replace(&supp->idr, INVALID_REQ_PTR, req->id); + ret = TEEC_ERROR_COMMUNICATION; } + mutex_unlock(&supp->mutex); - req->ret = TEEC_ERROR_COMMUNICATION; + } else { + ret = req->ret; } - ret = req->ret; kfree(req); return ret; } static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, - int num_params, int *id) + int num_params) { struct optee_supp_req *req; @@ -153,8 +184,8 @@ static struct optee_supp_req *supp_pop_entry(struct optee_supp *supp, return ERR_PTR(-EINVAL); } - *id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL); - if (*id < 0) + req->id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL); + if (req->id < 0) return ERR_PTR(-ENOMEM); list_del(&req->link); @@ -214,7 +245,6 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, struct optee *optee = tee_get_drvdata(teedev); struct optee_supp *supp = &optee->supp; struct optee_supp_req *req = NULL; - int id; size_t num_meta; int rc; @@ -224,15 +254,48 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, while (true) { mutex_lock(&supp->mutex); - req = supp_pop_entry(supp, *num_params - num_meta, &id); - mutex_unlock(&supp->mutex); - if (req) { - if (IS_ERR(req)) - return PTR_ERR(req); - break; + req = supp_pop_entry(supp, *num_params - num_meta); + if (!req) { + mutex_unlock(&supp->mutex); + goto wait_for_request; + } + + if (IS_ERR(req)) { + rc = PTR_ERR(req); + mutex_unlock(&supp->mutex); + + return rc; } + /* + * Process the request while holding the lock, so that + * optee_supp_thrd_req() doesn't pull the request from under us. + */ + + if (num_meta) { + /* + * tee-supplicant support meta parameters -> + * requests can be processed asynchronously. + */ + param->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT | + TEE_IOCTL_PARAM_ATTR_META; + param->u.value.a = req->id; + param->u.value.b = 0; + param->u.value.c = 0; + } else { + supp->req_id = req->id; + } + + *func = req->func; + *num_params = req->num_params + num_meta; + memcpy(param + num_meta, req->param, + sizeof(struct tee_param) * req->num_params); + + mutex_unlock(&supp->mutex); + return 0; + +wait_for_request: /* * If we didn't get a request we'll block in * wait_for_completion() to avoid needless spinning. @@ -243,29 +306,10 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, */ if (wait_for_completion_interruptible(&supp->reqs_c)) return -ERESTARTSYS; - } - if (num_meta) { - /* - * tee-supplicant support meta parameters -> requsts can be - * processed asynchronously. - */ - param->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT | - TEE_IOCTL_PARAM_ATTR_META; - param->u.value.a = id; - param->u.value.b = 0; - param->u.value.c = 0; - } else { - mutex_lock(&supp->mutex); - supp->req_id = id; - mutex_unlock(&supp->mutex); + /* Check for the next request in the queue. */ } - *func = req->func; - *num_params = req->num_params + num_meta; - memcpy(param + num_meta, req->param, - sizeof(struct tee_param) * req->num_params); - return 0; } @@ -297,12 +341,18 @@ static struct optee_supp_req *supp_pop_req(struct optee_supp *supp, if (!req) return ERR_PTR(-ENOENT); + /* optee_supp_thrd_req() already returned to optee. */ + if (IS_ERR(req)) + goto failed_req; + if ((num_params - nm) != req->num_params) return ERR_PTR(-EINVAL); + *num_meta = nm; +failed_req: idr_remove(&supp->idr, id); supp->req_id = -1; - *num_meta = nm; + return req; } @@ -328,9 +378,8 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params, mutex_lock(&supp->mutex); req = supp_pop_req(supp, num_params, param, &num_meta); - mutex_unlock(&supp->mutex); - if (IS_ERR(req)) { + mutex_unlock(&supp->mutex); /* Something is wrong, let supplicant restart. */ return PTR_ERR(req); } @@ -355,9 +404,10 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params, } } req->ret = ret; - + req->processed = true; /* Let the requesting thread continue */ complete(&req->c); + mutex_unlock(&supp->mutex); return 0; } --- base-commit: 3f24e4edcd1b8981c6b448ea2680726dedd87279 change-id: 20250604-fix-use-after-free-8ff1b5d5d774 Best regards, -- Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com>

3 months, 1 week

3
9
1 0

Linaro OP-TEE Contribution, LOC, monthly meeting January 27

by Jens Wiklander

Hi, Tomorrow, Tuesday, it's time for another LOC monthly meeting. For time and connection details, see the calendar at https://www.trustedfirmware.org/meetings/ - OP-TEE version 4.9.0 released - Following our discussion at the last LOC regarding exporting OP-TEE version information via sysfs, the latest version of the patch [1] has been accepted. - Any other topics? [1] https://lore.kernel.org/op-tee/20260112154833.78546-1-aristo.chen@canonical… Cheers, Jens

3 months, 3 weeks

1
0
0 0

Rebase linaro-swg/kernel to v6.18

by Jens Wiklander

Hi, I've just rebased the linaro-swg/kernel optee branch on v6.18 from v6.14. By rebasing, we eliminated a few workarounds on the optee branch and gained FF-A version 1.2 support in the kernel. Cheers, Jens

4 months

1
0
0 0

2026

2025

2024

2023

2022

2021

2020

OP-TEE