On Mon, Apr 18, 2016 at 11:51 PM, David Brown <david.brown@linaro.org> wrote:
> Looking at the TEE_Param definition:
>
> typedef union {
>         struct {
>                 void *buffer;
>                 uint32_t size;
>         } memref;
>         struct {
>                 uint32_t a;
>                 uint32_t b;
>         } value;
> } TEE_Param;
>
> there seems to be an implicit assumption of 32-bit pointers.  There
> are two things going on here:
>
>  - The 'memref' struct is larger than the 'value' structure, since
>    the 'void *' is 8-bytes long on a 64-bit target.
>
>  - The 'size' field is a uint32_t, which causes a pointer mismatch
>    with a lot of API calls that are looking for a size_t.
>
> The first is only an issue if either we are expecting this param to be
> a specific size, or we expect some kind of correlation between
> 'buffer' and 'a', and 'size' and 'b'.
>
> The second means that instead of being able to use
> &param[n].memref.size, it is necessary to do something like:
>
>         uint32_t local_size;
>
>         ...
>
>         local_size = param[n].memref.size;
>         result = call(..., &local_size, ...);
>         param[n].memref.size = local_size;

Yes, this is inconvenient.

>
> in addition to security concerns with overflow on the size fields.
>
> My question then, how do we want to handle this?  Assuming we want to
> make the TEE 64-bit compatible, what is the right answer?  As well,
> what do relevant standards/specs say about this type (the file right
> above this definition says based on "GP TEE Internal API Specification
> Version 0.11", which I have not seen).

The comment is out of date, we're now at version 1.1. The definition
is exactly as in the 1.1 spec.

We can address the overflow on the size field by checking the size
before filling in the parameter for the TA. Internally in OP-TEE we're
passing parameters to TAs via struct utee_params, which is later
translated into a TEE_Param before TA_InvokeCommandEntryPoint() and
TA_OpenSessionEntryPoint() are called. Then there's also
TEE_OpenTASession() TEE_InvokeTACommand() which also uses the
TEE_Param type.

I think we should raise this issue with GP.

Regards,
Jens

_______________________________________________
Tee-dev mailing list
Tee-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/tee-dev