[U-Boot] [PATCH v3 15/20] tee: add sandbox driver
Jens Wiklander
jens.wiklander at linaro.org
Tue Sep 25 07:33:41 UTC 2018
Hi Simon,
I have one question inline below.
On Fri, Sep 14, 2018 at 12:53 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Jens,
>
> On 3 September 2018 at 16:47, Jens Wiklander <jens.wiklander at linaro.org> wrote:
>> Adds a sandboxtee driver which emulates a generic TEE with the OP-TEE
>
> sandbox tee
>
>> AVB TA.
>>
>> Signed-off-by: Jens Wiklander <jens.wiklander at linaro.org>
>> ---
>> drivers/tee/Kconfig | 12 +-
>> drivers/tee/Makefile | 1 +
>> drivers/tee/optee/Kconfig | 2 +-
>> drivers/tee/sandbox.c | 299 ++++++++++++++++++++++++++++++++++++++
>> include/sandboxtee.h | 15 ++
>> 5 files changed, 326 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/tee/sandbox.c
>> create mode 100644 include/sandboxtee.h
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> nits below.
>
>>
>> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
>> index 835c256e9239..4697b80913d6 100644
>> --- a/drivers/tee/Kconfig
>> +++ b/drivers/tee/Kconfig
>> @@ -1,8 +1,8 @@
>> # Generic Trusted Execution Environment Configuration
>> config TEE
>> bool "Trusted Execution Environment support"
>> - depends on ARM && (ARM64 || CPU_V7A)
>> - select ARM_SMCCC
>> + depends on (ARM && (ARM64 || CPU_V7A)) || SANDBOX
>> + select ARM_SMCCC if ARM
>> help
>> This implements a generic interface towards a Trusted Execution
>> Environment (TEE). A TEE is a trusted OS running in some secure
>> @@ -14,6 +14,14 @@ if TEE
>>
>> menu "TEE drivers"
>>
>> +config SANDBOX_TEE
>> + bool "Sandbox TEE emulator"
>> + depends on SANDBOX
>> + default y
>> + help
>> + This emulates a generic TEE needed for testing including the AVB
>> + TA.
>
> Can you please expand this? What features does it implement? How do
> you access it from the U-Boot command line?
I'll expand it.
>
>> +
>> source "drivers/tee/optee/Kconfig"
>>
>> endmenu
>> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
>> index 19633b60f235..f72c68c09f33 100644
>> --- a/drivers/tee/Makefile
>> +++ b/drivers/tee/Makefile
>> @@ -1,4 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0+
>>
>> obj-y += tee-uclass.o
>> +obj-$(CONFIG_SANDBOX) += sandbox.o
>> obj-$(CONFIG_OPTEE) += optee/
>> diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
>> index dbfa7846a30f..d489834df926 100644
>> --- a/drivers/tee/optee/Kconfig
>> +++ b/drivers/tee/optee/Kconfig
>> @@ -10,7 +10,7 @@ config OPTEE
>> handle Remote Procedure Calls (RPC) from OP-TEE needed to
>> execute a service. For more information see: https://www.op-tee.org
>>
>> -if OPTEE
>> +if OPTEE || SANDBOX
>>
>> menu "OP-TEE options"
>>
>> diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c
>> new file mode 100644
>> index 000000000000..cd073497615f
>> --- /dev/null
>> +++ b/drivers/tee/sandbox.c
>> @@ -0,0 +1,299 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2018 Linaro Limited
>> + */
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <sandboxtee.h>
>
> Could this go in asm/ ?
I don't understand. What should go into asm/ and which directory is that?
>
>> +#include <tee.h>
>> +#include <tee/optee_ta_avb.h>
>> +
>> +/*
>> + * The sandbox tee driver tries to emulate a generic TEE with the TA
>> + * OPTEE_TA_AVB available.
>
> What is TEE?
>
> What is TA?
>
> Please expand out the names once in this comment so people can see
> immediately what you are referring to.
>
>> + */
>> +
>
>
>
>> +struct ta_entry {
>
> What is this struct for? Please add a comment
OK
>
>> + struct tee_optee_ta_uuid uuid;
>> + u32 (*open_session)(uint num_params, struct tee_param *params);
>> + u32 (*invoke_func)(u32 func, uint num_params, struct tee_param *params);
>> +};
>> +
>> +#ifdef CONFIG_OPTEE_TA_AVB
>> +static u32 get_attr(uint n, uint num_params, struct tee_param *params)
>> +{
>> + if (n >= num_params)
>> + return TEE_PARAM_ATTR_TYPE_NONE;
>> +
>> + return params[n].attr;
>> +}
>> +
>> +static u32 check_params(u8 p0, u8 p1, u8 p2, u8 p3, uint num_params,
>> + struct tee_param *params)
>> +{
>> + u8 p[] = { p0, p1, p2, p3};
>> + uint n;
>> +
>> + for (n = 0; n < ARRAY_SIZE(p); n++)
>> + if (p[n] != get_attr(n, num_params, params))
>> + goto bad_params;
>> +
>> + for (; n < num_params; n++)
>> + if (get_attr(n, num_params, params))
>> + goto bad_params;
>> +
>> + return TEE_SUCCESS;
>> +
>> +bad_params:
>> + printf("Bad param attrs\n");
>> +
>> + return TEE_ERROR_BAD_PARAMETERS;
>> +}
>> +
>> +static u64 ta_avb_rollback_indexes[TA_AVB_MAX_ROLLBACK_LOCATIONS];
>> +static u32 ta_avb_lock_state;
>> +
>> +static u32 ta_avb_open_session(uint num_params, struct tee_param *params)
>> +{
>> + /*
>> + * We don't expect additional parameters when opening a session to
>> + * this TA.
>> + */
>> + return check_params(TEE_PARAM_ATTR_TYPE_NONE, TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE, TEE_PARAM_ATTR_TYPE_NONE,
>> + num_params, params);
>> +}
>> +
>> +static u32 ta_avb_invoke_func(u32 func, uint num_params,
>> + struct tee_param *params)
>> +{
>> + u32 res;
>> + uint slot;
>> + u64 val;
>> +
>> + switch (func) {
>> + case TA_AVB_CMD_READ_ROLLBACK_INDEX:
>> + res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,
>> + TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + num_params, params);
>> + if (res)
>> + return res;
>> +
>> + slot = params[0].u.value.a;
>> + if (slot >= ARRAY_SIZE(ta_avb_rollback_indexes)) {
>> + printf("Rollback index slot out of bounds %lu\n", slot);
>> + return TEE_ERROR_BAD_PARAMETERS;
>> + }
>> +
>> + val = ta_avb_rollback_indexes[slot];
>> + params[1].u.value.a = val >> 32;
>> + params[1].u.value.b = val;
>> + return TEE_SUCCESS;
>> +
>> + case TA_AVB_CMD_WRITE_ROLLBACK_INDEX:
>> + res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,
>> + TEE_PARAM_ATTR_TYPE_VALUE_INPUT,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + num_params, params);
>> + if (res)
>> + return res;
>> +
>> + slot = params[0].u.value.a;
>> + if (slot >= ARRAY_SIZE(ta_avb_rollback_indexes)) {
>> + printf("Rollback index slot out of bounds %lu\n", slot);
>> + return TEE_ERROR_BAD_PARAMETERS;
>> + }
>> +
>> + val = (u64)params[1].u.value.a << 32 | params[1].u.value.b;
>> + if (val < ta_avb_rollback_indexes[slot])
>> + return TEE_ERROR_SECURITY;
>> +
>> + ta_avb_rollback_indexes[slot] = val;
>> + return TEE_SUCCESS;
>> +
>> + case TA_AVB_CMD_READ_LOCK_STATE:
>> + res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + num_params, params);
>> + if (res)
>> + return res;
>> +
>> + params[0].u.value.a = ta_avb_lock_state;
>> + return TEE_SUCCESS;
>> +
>> + case TA_AVB_CMD_WRITE_LOCK_STATE:
>> + res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + TEE_PARAM_ATTR_TYPE_NONE,
>> + num_params, params);
>> + if (res)
>> + return res;
>> +
>> + if (ta_avb_lock_state != params[0].u.value.a) {
>> + ta_avb_lock_state = params[0].u.value.a;
>> + memset(ta_avb_rollback_indexes, 0,
>> + sizeof(ta_avb_rollback_indexes));
>> + }
>> +
>> + return TEE_SUCCESS;
>> +
>> + default:
>> + return TEE_ERROR_NOT_SUPPORTED;
>> + }
>> +}
>> +#endif /*OPTEE_TA_AVB*/
>> +
>> +static const struct ta_entry ta_entries[] = {
>> +#ifdef CONFIG_OPTEE_TA_AVB
>> + { .uuid = TA_AVB_UUID,
>> + .open_session = ta_avb_open_session,
>> + .invoke_func = ta_avb_invoke_func,
>> + },
>> +#endif
>> +};
>> +
>> +static void sandbox_tee_get_version(struct udevice *dev,
>> + struct tee_version_data *vers)
>> +{
>> + struct tee_version_data v = {
>> + .gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_REG_MEM,
>> + };
>> +
>> + *vers = v;
>> +}
>> +
>> +static int sandbox_tee_close_session(struct udevice *dev, u32 session)
>> +{
>> + struct sandbox_tee_state *state = dev_get_priv(dev);
>> +
>> + if (!state->ta || state->session != session)
>> + return -EINVAL;
>> +
>> + state->session = 0;
>> + state->ta = NULL;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct ta_entry *find_ta_entry(u8 uuid[TEE_UUID_LEN])
>> +{
>> + struct tee_optee_ta_uuid u;
>> + uint n;
>> +
>> + tee_optee_ta_uuid_from_octets(&u, uuid);
>> +
>> + for (n = 0; n < ARRAY_SIZE(ta_entries); n++)
>> + if (!memcmp(&u, &ta_entries[n].uuid, sizeof(u)))
>> + return ta_entries + n;
>> +
>> + return NULL;
>> +}
>> +
>> +static int sandbox_tee_open_session(struct udevice *dev,
>> + struct tee_open_session_arg *arg,
>> + uint num_params, struct tee_param *params)
>> +{
>> + struct sandbox_tee_state *state = dev_get_priv(dev);
>> + const struct ta_entry *ta;
>> +
>> + if (state->ta) {
>> + printf("A session is already open\n");
>> + return -EBUSY;
>> + }
>> +
>> + ta = find_ta_entry(arg->uuid);
>> + if (!ta) {
>> + printf("Cannot find TA\n");
>> + arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
>> + arg->ret_origin = TEE_ORIGIN_TEE;
>> +
>> + return 0;
>> + }
>> +
>> + arg->ret = ta->open_session(num_params, params);
>> + arg->ret_origin = TEE_ORIGIN_TRUSTED_APP;
>> +
>> + if (!arg->ret) {
>> + state->ta = (void *)ta;
>> + state->session = 1;
>> + arg->session = state->session;
>> + } else {
>> + printf("Cannot open session, TA returns error\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int sandbox_tee_invoke_func(struct udevice *dev,
>> + struct tee_invoke_arg *arg,
>> + uint num_params, struct tee_param *params)
>> +{
>> + struct sandbox_tee_state *state = dev_get_priv(dev);
>> + struct ta_entry *ta = state->ta;
>> +
>> + if (!arg->session) {
>> + printf("Missing session\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (!ta) {
>> + printf("TA session not available\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (arg->session != state->session) {
>> + printf("Session mismatch\n");
>> + return -EINVAL;
>> + }
>> +
>> + arg->ret = ta->invoke_func(arg->func, num_params, params);
>> + arg->ret_origin = TEE_ORIGIN_TRUSTED_APP;
>> +
>> + return 0;
>> +}
>> +
>> +static int sandbox_tee_shm_register(struct udevice *dev, struct tee_shm *shm)
>> +{
>> + struct sandbox_tee_state *state = dev_get_priv(dev);
>> +
>> + state->num_shms++;
>> +
>> + return 0;
>> +}
>> +
>> +static int sandbox_tee_shm_unregister(struct udevice *dev, struct tee_shm *shm)
>> +{
>> + struct sandbox_tee_state *state = dev_get_priv(dev);
>> +
>> + state->num_shms--;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct tee_driver_ops sandbox_tee_ops = {
>> + .get_version = sandbox_tee_get_version,
>> + .open_session = sandbox_tee_open_session,
>> + .close_session = sandbox_tee_close_session,
>> + .invoke_func = sandbox_tee_invoke_func,
>> + .shm_register = sandbox_tee_shm_register,
>> + .shm_unregister = sandbox_tee_shm_unregister,
>> +};
>> +
>> +static const struct udevice_id sandbox_tee_match[] = {
>> + { .compatible = "sandbox,tee" },
>> + {},
>> +};
>> +
>> +U_BOOT_DRIVER(sandbox_tee) = {
>> + .name = "sandbox_tee",
>> + .id = UCLASS_TEE,
>> + .of_match = sandbox_tee_match,
>> + .ops = &sandbox_tee_ops,
>> + .priv_auto_alloc_size = sizeof(struct sandbox_tee_state),
>> +};
>> diff --git a/include/sandboxtee.h b/include/sandboxtee.h
>> new file mode 100644
>> index 000000000000..59cbb621820b
>> --- /dev/null
>> +++ b/include/sandboxtee.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright (C) 2018 Linaro Limited
>> + */
>> +
>> +#ifndef __SANDBOXTEE_H
>> +#define __SANDBOXTEE_H
>> +
>> +struct sandbox_tee_state {
>> + u32 session;
>> + int num_shms;
>> + void *ta;
>
> struct/members need comments
OK
Thanks,
Jens
More information about the U-Boot
mailing list