2021-04-20 08:27:35

by Jens Wiklander

[permalink] [raw]
Subject: [PATCH] optee: use export_uuid() to copy client UUID

Prior to this patch optee_open_session() was making assumptions about
the internal format of uuid_t by casting a memory location in a
parameter struct to uuid_t *. Fix this using export_uuid() to get a well
defined binary representation and also add an octets field in struct
optee_msg_param in order to avoid casting.

Fixes: c5b4312bea5d ("tee: optee: Add support for session login client UUID generation")
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
---
drivers/tee/optee/call.c | 6 ++++--
drivers/tee/optee/optee_msg.h | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index 7a77e375b503..6b52f0c526ba 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -216,6 +216,7 @@ int optee_open_session(struct tee_context *ctx,
struct optee_msg_arg *msg_arg;
phys_addr_t msg_parg;
struct optee_session *sess = NULL;
+ uuid_t client_uuid;

/* +2 for the meta parameters added below */
shm = get_msg_arg(ctx, arg->num_params + 2, &msg_arg, &msg_parg);
@@ -236,10 +237,11 @@ int optee_open_session(struct tee_context *ctx,
memcpy(&msg_arg->params[0].u.value, arg->uuid, sizeof(arg->uuid));
msg_arg->params[1].u.value.c = arg->clnt_login;

- rc = tee_session_calc_client_uuid((uuid_t *)&msg_arg->params[1].u.value,
- arg->clnt_login, arg->clnt_uuid);
+ rc = tee_session_calc_client_uuid(&client_uuid, arg->clnt_login,
+ arg->clnt_uuid);
if (rc)
goto out;
+ export_uuid(msg_arg->params[1].u.octets, &client_uuid);

rc = optee_to_msg_param(msg_arg->params + 2, arg->num_params, param);
if (rc)
diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
index 81ff593ac4ec..e3d72d09c484 100644
--- a/drivers/tee/optee/optee_msg.h
+++ b/drivers/tee/optee/optee_msg.h
@@ -9,7 +9,7 @@
#include <linux/types.h>

/*
- * This file defines the OP-TEE message protocol used to communicate
+ * This file defines the OP-TEE message protocol (ABI) used to communicate
* with an instance of OP-TEE running in secure world.
*
* This file is divided into two sections.
@@ -144,9 +144,10 @@ struct optee_msg_param_value {
* @tmem: parameter by temporary memory reference
* @rmem: parameter by registered memory reference
* @value: parameter by opaque value
+ * @octets: parameter by octet string
*
* @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
- * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value,
+ * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value or octets,
* OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
* OPTEE_MSG_ATTR_TYPE_RMEM_* indicates @rmem,
* OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
@@ -157,6 +158,7 @@ struct optee_msg_param {
struct optee_msg_param_tmem tmem;
struct optee_msg_param_rmem rmem;
struct optee_msg_param_value value;
+ u8 octets[24];
} u;
};

--
2.25.1


2021-04-20 10:28:17

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] optee: use export_uuid() to copy client UUID

On Tue, Apr 20, 2021 at 10:25:47AM +0200, Jens Wiklander wrote:
> Prior to this patch optee_open_session() was making assumptions about
> the internal format of uuid_t by casting a memory location in a
> parameter struct to uuid_t *. Fix this using export_uuid() to get a well
> defined binary representation and also add an octets field in struct
> optee_msg_param in order to avoid casting.

Wonderful! Thanks for fixing this!
Reviewed-by: Andy Shevchenko <[email protected]>

A bit of off-topic, have you know by any chance who may consider applying this
one?
https://lore.kernel.org/linux-mips/[email protected]/

> Fixes: c5b4312bea5d ("tee: optee: Add support for session login client UUID generation")
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Jens Wiklander <[email protected]>
> ---
> drivers/tee/optee/call.c | 6 ++++--
> drivers/tee/optee/optee_msg.h | 6 ++++--
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
> index 7a77e375b503..6b52f0c526ba 100644
> --- a/drivers/tee/optee/call.c
> +++ b/drivers/tee/optee/call.c
> @@ -216,6 +216,7 @@ int optee_open_session(struct tee_context *ctx,
> struct optee_msg_arg *msg_arg;
> phys_addr_t msg_parg;
> struct optee_session *sess = NULL;
> + uuid_t client_uuid;
>
> /* +2 for the meta parameters added below */
> shm = get_msg_arg(ctx, arg->num_params + 2, &msg_arg, &msg_parg);
> @@ -236,10 +237,11 @@ int optee_open_session(struct tee_context *ctx,
> memcpy(&msg_arg->params[0].u.value, arg->uuid, sizeof(arg->uuid));
> msg_arg->params[1].u.value.c = arg->clnt_login;
>
> - rc = tee_session_calc_client_uuid((uuid_t *)&msg_arg->params[1].u.value,
> - arg->clnt_login, arg->clnt_uuid);
> + rc = tee_session_calc_client_uuid(&client_uuid, arg->clnt_login,
> + arg->clnt_uuid);
> if (rc)
> goto out;
> + export_uuid(msg_arg->params[1].u.octets, &client_uuid);
>
> rc = optee_to_msg_param(msg_arg->params + 2, arg->num_params, param);
> if (rc)
> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> index 81ff593ac4ec..e3d72d09c484 100644
> --- a/drivers/tee/optee/optee_msg.h
> +++ b/drivers/tee/optee/optee_msg.h
> @@ -9,7 +9,7 @@
> #include <linux/types.h>
>
> /*
> - * This file defines the OP-TEE message protocol used to communicate
> + * This file defines the OP-TEE message protocol (ABI) used to communicate
> * with an instance of OP-TEE running in secure world.
> *
> * This file is divided into two sections.
> @@ -144,9 +144,10 @@ struct optee_msg_param_value {
> * @tmem: parameter by temporary memory reference
> * @rmem: parameter by registered memory reference
> * @value: parameter by opaque value
> + * @octets: parameter by octet string
> *
> * @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
> - * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value,
> + * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value or octets,
> * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
> * OPTEE_MSG_ATTR_TYPE_RMEM_* indicates @rmem,
> * OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
> @@ -157,6 +158,7 @@ struct optee_msg_param {
> struct optee_msg_param_tmem tmem;
> struct optee_msg_param_rmem rmem;
> struct optee_msg_param_value value;
> + u8 octets[24];
> } u;
> };
>
> --
> 2.25.1
>

--
With Best Regards,
Andy Shevchenko