diff options
| author | Tom Rini <[email protected]> | 2024-04-18 12:13:40 -0600 |
|---|---|---|
| committer | Tom Rini <[email protected]> | 2024-04-18 12:13:40 -0600 |
| commit | d893c93205701b77f9b3f2e8074297a32d8710db (patch) | |
| tree | 4cf5108b5791612b65d21fb649114345bee70a32 | |
| parent | cdf0195e90b66f25ed44fa5ed5634ec064e8dcb9 (diff) | |
| parent | b905599b36e3d8158c5cd045c26278416909b422 (diff) | |
Merge tag 'tpm-master-18042024' of https://source.denx.de/u-boot/custodians/u-boot-tpm
Igor says:
"The problem initially was in the TEE sandbox driver implementation
(drivers/tee/sandbox.c) and it's limitations, which doesn't
permit to have multiple simultaneous sessions with different TAs.
This is what actually happened in this CI run [1], firstly "optee_rpmb"
cmd was executed (and after execution we had one session open), and
then "scp03", which also makes calls to OP-TEE, however it fails
in sandbox_tee_open_session() because of this check:
if (state->ta) {
printf("A session is already open\n");
return -EBUSY;
}
I had two ways in mind to address that:
1. Close a session on each optee_rpmb cmd invocation.
I don't see any reason to keep this session open, as obviously
there is no other mechanism (tbh, I don't know if DM calls ".remove" for active
devices) to close it automatically before handing over control to
Linux kernel. As a result we might end up with some orphaned sessions
registered in OP-TEE OS core (obvious resource leak).
2. Extend TEE sandbox driver, add support for multiple
simultaneous sessions just to handle the case.
I've chosen the first approach, as IMO it was "kill two birds with one stone",
I could address resource leak in OP-TEE and bypass limitations of
TEE sandbox driver."
Link: https://lore.kernel.org/u-boot/CAByghJZVRbnFUwJdgU534tvGA+DX2pArf0i7ySik=BrXgADe3Q@mail.gmail.com/
The CI https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/20414
showed no problems
| -rw-r--r-- | cmd/Kconfig | 4 | ||||
| -rw-r--r-- | cmd/optee_rpmb.c | 23 | ||||
| -rw-r--r-- | drivers/tee/broadcom/chimp_optee.c | 3 | ||||
| -rw-r--r-- | drivers/tee/optee/Kconfig | 2 | ||||
| -rw-r--r-- | drivers/tee/optee/core.c | 1 | ||||
| -rw-r--r-- | drivers/tee/optee/i2c.c | 1 | ||||
| -rw-r--r-- | drivers/tee/optee/rpmb.c | 1 | ||||
| -rw-r--r-- | drivers/tee/optee/supplicant.c | 2 | ||||
| -rw-r--r-- | drivers/tee/sandbox.c | 2 | ||||
| -rw-r--r-- | drivers/tee/tee-uclass.c | 1 | ||||
| -rw-r--r-- | test/py/tests/test_optee_rpmb.py | 20 |
11 files changed, 45 insertions, 15 deletions
diff --git a/cmd/Kconfig b/cmd/Kconfig index 126bdeeb6d2..408cc84c182 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1396,7 +1396,9 @@ config CMD_CLONE config CMD_OPTEE_RPMB bool "Enable read/write support on RPMB via OPTEE" - depends on SUPPORT_EMMC_RPMB && OPTEE + depends on (SUPPORT_EMMC_RPMB && OPTEE) || SANDBOX_TEE + default y if SANDBOX_TEE + select OPTEE_TA_AVB if SANDBOX_TEE help Enable the commands for reading, writing persistent named values in the Replay Protection Memory Block partition in eMMC by diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c index e0e44bbed04..b3cafd92410 100644 --- a/cmd/optee_rpmb.c +++ b/cmd/optee_rpmb.c @@ -87,8 +87,10 @@ static int read_persistent_value(const char *name, rc = tee_shm_alloc(tee, name_size, TEE_SHM_ALLOC, &shm_name); - if (rc) - return -ENOMEM; + if (rc) { + rc = -ENOMEM; + goto close_session; + } rc = tee_shm_alloc(tee, buffer_size, TEE_SHM_ALLOC, &shm_buf); @@ -125,6 +127,9 @@ out: tee_shm_free(shm_buf); free_name: tee_shm_free(shm_name); +close_session: + tee_close_session(tee, session); + tee = NULL; return rc; } @@ -139,17 +144,20 @@ static int write_persistent_value(const char *name, struct tee_param param[2]; size_t name_size = strlen(name) + 1; + if (!value_size) + return -EINVAL; + if (!tee) { if (avb_ta_open_session()) return -ENODEV; } - if (!value_size) - return -EINVAL; rc = tee_shm_alloc(tee, name_size, TEE_SHM_ALLOC, &shm_name); - if (rc) - return -ENOMEM; + if (rc) { + rc = -ENOMEM; + goto close_session; + } rc = tee_shm_alloc(tee, value_size, TEE_SHM_ALLOC, &shm_buf); @@ -178,6 +186,9 @@ out: tee_shm_free(shm_buf); free_name: tee_shm_free(shm_name); +close_session: + tee_close_session(tee, session); + tee = NULL; return rc; } diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c index 37f9b094f76..bd146ef2899 100644 --- a/drivers/tee/broadcom/chimp_optee.c +++ b/drivers/tee/broadcom/chimp_optee.c @@ -3,9 +3,10 @@ * Copyright 2020 Broadcom. */ -#include <common.h> #include <tee.h> #include <broadcom/chimp.h> +#include <linux/errno.h> +#include <string.h> #ifdef CONFIG_CHIMP_OPTEE diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index 9dc65b0501e..db0bcfa6f15 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -19,7 +19,7 @@ config OPTEE_TA_AVB default y help Enables support for the AVB Trusted Application (TA) in OP-TEE. - The TA can support the "avb" subcommands "read_rb", "write"rb" + The TA can support the "avb" subcommands "read_rb", "write_rb" and "is_unlocked". config OPTEE_TA_RPC_TEST diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 47f845cffe3..5fc0505c788 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -3,7 +3,6 @@ * Copyright (c) 2018-2020 Linaro Limited */ -#include <common.h> #include <cpu_func.h> #include <dm.h> #include <dm/device_compat.h> diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c index ef4e10f9912..e3fb99897c5 100644 --- a/drivers/tee/optee/i2c.c +++ b/drivers/tee/optee/i2c.c @@ -3,7 +3,6 @@ * Copyright (c) 2020 Foundries.io Ltd */ -#include <common.h> #include <dm.h> #include <i2c.h> #include <tee.h> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 5bc13757ea8..bacced6af6c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -3,7 +3,6 @@ * Copyright (c) 2018 Linaro Limited */ -#include <common.h> #include <dm.h> #include <log.h> #include <tee.h> diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index f9dd874b594..8a426f53ba8 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -3,10 +3,10 @@ * Copyright (c) 2018, Linaro Limited */ -#include <common.h> #include <log.h> #include <malloc.h> #include <tee.h> +#include <linux/errno.h> #include <linux/types.h> #include "optee_msg.h" diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index ec66401878c..8ad7c09efdd 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2018 Linaro Limited */ -#include <common.h> + #include <dm.h> #include <sandboxtee.h> #include <tee.h> diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c index 52412a4098e..0194d732193 100644 --- a/drivers/tee/tee-uclass.c +++ b/drivers/tee/tee-uclass.c @@ -5,7 +5,6 @@ #define LOG_CATEGORY UCLASS_TEE -#include <common.h> #include <cpu_func.h> #include <dm.h> #include <log.h> diff --git a/test/py/tests/test_optee_rpmb.py b/test/py/tests/test_optee_rpmb.py new file mode 100644 index 00000000000..8a081b5c494 --- /dev/null +++ b/test/py/tests/test_optee_rpmb.py @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Tests for OP-TEE RPMB read/write support + +""" +This tests optee_rpmb cmd in U-Boot +""" + +import pytest +import u_boot_utils as util + [email protected]('cmd_optee_rpmb') +def test_optee_rpmb_read_write(u_boot_console): + """Test OP-TEE RPMB cmd read/write + """ + response = u_boot_console.run_command('optee_rpmb write_pvalue test_variable test_value') + assert response == 'Wrote 11 bytes' + + response = u_boot_console.run_command('optee_rpmb read_pvalue test_variable 11') + assert response == 'Read 11 bytes, value = test_value'
\ No newline at end of file |
