Re: [PATCH] xen/arm: optee: allow plain TMEM buffers with NULL address

[Julien Grall <julien@xxxxxxx>]

On 19/06/2020 00:29, Volodymyr Babchuk wrote:
> Hi Julien,
Hi Volodymyr,

> Julien Grall writes:
>
> > (+Paul)
> >
> > Hi,
> >
> > On 18/05/2020 02:53, Volodymyr Babchuk wrote:
> > > Trusted Applications use popular approach to determine required size
> > > of buffer: client provides a memory reference with the NULL pointer to
> > > a buffer. This is so called "Null memory reference".  TA updates the
> > NIT: You use double space after '.' here but all the others use a
> > single space.
> Oops, missed that.
>
> > > reference with the required size and returns it back to client. Then
> > > client allocates buffer of needed size and repeats the operation.
> > >
> > > This behavior is described in TEE Client API Specification, paragraph
> > > 3.2.5. Memory References.
> >  From the spec, it is not a clear cut that NULL will always used as
> > fetching the required size of an output buffer. In particular, they
> > suggest to refer to the protocol.
> >
> > In your commit message you indirectly point to an example where 0/NULL
> > would have a different meaning depending on the flags. This is not
> > explained in the TEE Client API Specification. Do you have some
> > pointer I could use to check the behavior?
> Well, GP specification describes application interface. It does not
> specifies how implementation should handle this internally. Basically,
> GP defines functions, data types, macros, etc to be used by CAs and
> TAs. But it does not define how exactly call from CA will be delivered
> to TA. Implementation is free to use any means as far, as they conform
> with rules described in the specification.
>
> OPTEE's REE<->TEE interface is described in the header files, that I
> added to xen (optee_msg.h, optee_rpc_cmd.h,optee_smc.h). But it does not
> describe every aspect, unfortunately.
Thanks for digging-through! More below.

> > > OP-TEE represents this null memory reference as a TMEM parameter with
> > > buf_ptr == NULL. This is the only case when we should allow TMEM
> > > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag.
> > IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer
> > at address 0" but with the flag cleared, it would mean "return the
> > size". Am I correct?
> Not exactly. This flag does not affect behavior for buffers with address
> NULL. In any case, this would not mean "return the size" to
> OP-TEE. This is because OP-TEE works as a transport between CA and TA
> and it does not assign any meaning to client buffers. But certainly,
> null buffer will mean "return the size" for some TAs, as described in
> GlobalPlatform specification.
Does it mean a guest TA may potentially access address 0?

I am asking that because 0 can be a valid host physical address. We are 
currently carving out 0 from the heap allocator to workaround a bug, but 
there is no promise this address will be used by the boot allocator and 
therefore Xen.
> Reason why I prohibited buffers without OPTEE_MSG_ATTR_NONCONTIG flag in
> the the original patch is that such buffers could span across page
> boundary, which is no-go for fragmented p2m.
>
> But I missed that special case: null buffer without
> OPTEE_MSG_ATTR_NONCONTIG flag. As this is a special type of buffer, it
> can be allowed with and without said flag.
Looking at translate_noncontig(), there is no specific treatment for 
NULL. So the address will get translated. This is likely to result to an 
error as usually a guest doesn't have anything mapped at address 0.
Did I miss anything?

Cheers,

--
Julien Grall