On 19/06/2020 18:15, Stefano Stabellini wrote:
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.
I would disagree here. NULL has never been handled correctly for TMEM
buffers (see [1]). I would argue this needs to be folded within this
patch rather than be a separate one.
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?
+1 for the link. However, I don't think the commit message fully
match/summarize the discussion here.
What needs to be clearly spell out is that existing OP-TEEs will never
map MFN 0.
Cheers,
[1] <87h7v71ixf....@epam.com>
--
Julien Grall