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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 19 Jun 2020, Julien Grall wrote:
> Hi Volodymyr,
> 
> On 19/06/2020 10:52, Volodymyr Babchuk wrote:
> > > > > > 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?
> > 
> > TA is running in S-EL0. That buffer with PA=0x0 will be not mapped in TA
> > address space at all. So, if TA will try to access address 0, it
> > will be terminated with data abort.
> > 
> > > 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.
> > > 
> > 
> > Well, this is a potential issue in OP-TEE. It does not treat 0 as a
> > valid address. So, in that rare case, when REE will try to use 0
> > as a correct address for data buffer, OP-TEE will not map it into
> > TA address space, instead it will pass NULL to TA, so TA will think that
> > no buffer was provided.
> 
> Thanks! That's reassuring. Although, I think we may still need to prevent MFN
> 0 to be allocated for a guest using OP-TEE. So they don't end up with weird
> failure.
> 
> I don't think this is an issue so far, but this may change with Stefano's
> dom0less series to support direct mapping.

Yes, it is interesting to know about this limitation.

In regards to this patch, it looks OK as-is in terms of code changes.
Aside from a link to this conversation in the xen-devel archives, is
there anything else you would like to add to the commit message?