2021-03-22 10:43:46

by Jerome Forissier

[permalink] [raw]
Subject: [PATCH 1/1] tee: optee: do not check memref size on return from Secure World

When Secure World returns, it may have changed the size attribute of the
memory references passed as [in/out] parameters. The GlobalPlatform TEE
Internal Core API specification does not restrict the values that this
size can take. In particular, Secure World may increase the value to be
larger than the size of the input buffer to indicate that it needs more.

Therefore, the size check in optee_from_msg_param() is incorrect and
needs to be removed. This fixes a number of failed test cases in the
GlobalPlatform TEE Initial Configuratiom Test Suite v2_0_0_0-2017_06_09
when OP-TEE is compiled without dynamic shared memory support
(CFG_CORE_DYN_SHM=n).

Suggested-by: Jens Wiklander <[email protected]>
Signed-off-by: Jerome Forissier <[email protected]>
---
drivers/tee/optee/core.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 319a1e701163..ddb8f9ecf307 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -79,16 +79,6 @@ int optee_from_msg_param(struct tee_param *params, size_t num_params,
return rc;
p->u.memref.shm_offs = mp->u.tmem.buf_ptr - pa;
p->u.memref.shm = shm;
-
- /* Check that the memref is covered by the shm object */
- if (p->u.memref.size) {
- size_t o = p->u.memref.shm_offs +
- p->u.memref.size - 1;
-
- rc = tee_shm_get_pa(shm, o, NULL);
- if (rc)
- return rc;
- }
break;
case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT:
case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT:
--
2.25.1


2021-03-25 14:21:23

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH 1/1] tee: optee: do not check memref size on return from Secure World

On Mon, 22 Mar 2021 at 16:11, Jerome Forissier via OP-TEE
<[email protected]> wrote:
>
> When Secure World returns, it may have changed the size attribute of the
> memory references passed as [in/out] parameters. The GlobalPlatform TEE
> Internal Core API specification does not restrict the values that this
> size can take. In particular, Secure World may increase the value to be
> larger than the size of the input buffer to indicate that it needs more.
>
> Therefore, the size check in optee_from_msg_param() is incorrect and
> needs to be removed. This fixes a number of failed test cases in the
> GlobalPlatform TEE Initial Configuratiom Test Suite v2_0_0_0-2017_06_09
> when OP-TEE is compiled without dynamic shared memory support
> (CFG_CORE_DYN_SHM=n).
>
> Suggested-by: Jens Wiklander <[email protected]>
> Signed-off-by: Jerome Forissier <[email protected]>
> ---
> drivers/tee/optee/core.c | 10 ----------
> 1 file changed, 10 deletions(-)
>

Looks good to me.

Reviewed-by: Sumit Garg <[email protected]>

-Sumit

> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 319a1e701163..ddb8f9ecf307 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -79,16 +79,6 @@ int optee_from_msg_param(struct tee_param *params, size_t num_params,
> return rc;
> p->u.memref.shm_offs = mp->u.tmem.buf_ptr - pa;
> p->u.memref.shm = shm;
> -
> - /* Check that the memref is covered by the shm object */
> - if (p->u.memref.size) {
> - size_t o = p->u.memref.shm_offs +
> - p->u.memref.size - 1;
> -
> - rc = tee_shm_get_pa(shm, o, NULL);
> - if (rc)
> - return rc;
> - }
> break;
> case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT:
> case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT:
> --
> 2.25.1
>

2021-04-01 07:42:32

by Jens Wiklander

[permalink] [raw]
Subject: Re: [PATCH 1/1] tee: optee: do not check memref size on return from Secure World

On Thu, Mar 25, 2021 at 3:18 PM Sumit Garg <[email protected]> wrote:
>
> On Mon, 22 Mar 2021 at 16:11, Jerome Forissier via OP-TEE
> <[email protected]> wrote:
> >
> > When Secure World returns, it may have changed the size attribute of the
> > memory references passed as [in/out] parameters. The GlobalPlatform TEE
> > Internal Core API specification does not restrict the values that this
> > size can take. In particular, Secure World may increase the value to be
> > larger than the size of the input buffer to indicate that it needs more.
> >
> > Therefore, the size check in optee_from_msg_param() is incorrect and
> > needs to be removed. This fixes a number of failed test cases in the
> > GlobalPlatform TEE Initial Configuratiom Test Suite v2_0_0_0-2017_06_09
> > when OP-TEE is compiled without dynamic shared memory support
> > (CFG_CORE_DYN_SHM=n).
> >
> > Suggested-by: Jens Wiklander <[email protected]>
> > Signed-off-by: Jerome Forissier <[email protected]>
> > ---
> > drivers/tee/optee/core.c | 10 ----------
> > 1 file changed, 10 deletions(-)
> >
>
> Looks good to me.
>
> Reviewed-by: Sumit Garg <[email protected]>

Thanks, I'm picking this up.

Cheers,
Jens