[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Julien Grall <julien@xxxxxxx>
  • From: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Date: Fri, 19 Jun 2020 09:52:45 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nSAIFLl/JBZVYKwQ11W2i/ROCM2BixfB8A+9m3RJ5+8=; b=HrhTySdye83u1NJuUIszlgs/dcSaG30Cw8wdX1dTiIXzIyoXpffSmLE+1NC8ASJ/0jIimkH5ixAqgyppQwciaaCVm8ZGV49SHAwZhIjt/k3+t2TidAhbcaf+aTGIm8TG4B1EKe2ROU+JKF/AxjSoLBlcXWTcBwWu2LNsgxAgndyWPNHSPL4odsH0iFHML8sta2LUIIwGHT5IRZhxItwboEPYjVn9mQhEsT2fTxDlAqPJ1MrivxStaw308qEiPq+lDzseBYScJYAmVsnOPcjGrMIw89ZF08HTy0gpF9k5T5RSv0k2QWJnAsUNrN+584atTPwK7GQN+heNRFiqo1jjKQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=W38mVwoQgw6tlH2M9eISPcmDD8zzMh1+K3aEPRWODi5lN3WhbHj1FbiJH2hI9hDK/WIevVp2pCG0u77GYiwVh4dH1rbi7U/aOdhE9qHPymmKXBO0p6SGKaQa8uOQzvZfpx4DeCxU3pq3eIbpiRpsp1okDRb4ATTMOi9v0UMchQkT+8PHmWT+c8TpDig1PzDyL/dGdhz0JBAXnE5sYOTDUXik7fywXPOT1wCRsiGmOS3X6/z3XHJzMqxCqebH8Kx8aQNM3ivOR2so1LdIFo3r7kBJLriWM9FCcZaufDoZsrFZ4rjhtEw57IaxD0tDwBkWxP35wTiVK0oWpa1ngb36xQ==
  • Authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=epam.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "paul@xxxxxxx" <paul@xxxxxxx>
  • Delivery-date: Fri, 19 Jun 2020 09:52:54 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHWLLcTxhZtogukPEOG78aedSZTLajL0lyAgBNkugCAAKX1gIAACEgA
  • Thread-topic: [PATCH] xen/arm: optee: allow plain TMEM buffers with NULL address

Julien Grall writes:

> 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?

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.

>> 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.

Yes, you are right. Right now, optee client driver will not emit buffer
wit OPTEE_MSG_ATTR_NONCONTIG and address 0. But this is possible. And
this should be handled correctly. I'll add fix for that. Today, as
well. Thanks for pointing this out.

-- 
Volodymyr Babchuk at EPAM


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.