Re: [PATCH v4 06/10] xen/arm: optee: add support for RPC SHM buffers

From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
To: Julien Grall <julien.grall@arm.com>
Cc: "tee-dev@lists.linaro.org" <tee-dev@lists.linaro.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Volodymyr Babchuk <vlad.babchuk@gmail.com>
Subject: Re: [PATCH v4 06/10] xen/arm: optee: add support for RPC SHM buffers
Date: Wed, 20 Mar 2019 17:09:11 +0000	[thread overview]
Message-ID: <87k1gt8ocp.fsf@epam.com> (raw)
In-Reply-To: <b9adde4b-acd1-23d4-74b6-8807ba060d87@arm.com>

Julien Grall writes:

> Hi,
>
> On 20/03/2019 16:21, Volodymyr Babchuk wrote:
>>
>> Julien Grall writes:
>>> On 07/03/2019 21:04, Volodymyr Babchuk wrote:
>>>> From: Volodymyr Babchuk <vlad.babchuk@gmail.com>
>>>>
>>>> OP-TEE usually uses the same idea with command buffers (see
>>>> previous commit) to issue RPC requests. Problem is that initially
>>>> it has no buffer, where it can write request. So the first RPC
>>>> request it makes is special: it requests NW to allocate shared
>>>> buffer for other RPC requests. Usually this buffer is allocated
>>>> only once for every OP-TEE thread and it remains allocated all
>>>> the time until guest shuts down. Guest can ask OP-TEE to disable
>>>> RPC buffers caching, in this case OP-TEE will ask guest to
>>>> allocate/free buffer for the each RPC.
>>>>
>>>> Mediator needs to pin this buffer to make sure that domain can't
>>>> transfer it to someone else.
>>>
>>> At the moment, Xen on Arm doesn't support transfer of a page between
>>> domain (see steal_page). What we want to prevent here is the domain to
>>> free the page (via XENMEM_decrease_reservation). If the reference drop
>>> to 0, the page will be freed and could potentially be allocated for
>>> Xen usage or another domain. Taking the reference here, will prevent
>>> it to free until the reference is dropped.
>>>
>>> So I would reword this sentence. Something like:
>>>
>>> "Mediator needs to pin the buffer to make sure the page will not be
>>> freed while it is shared with OP-TEE".
>> When I wrote that, I kept in mind grant mappings as well. But, I
>> think, I was mistaken there, because grant mappings will have p2m type
>> p2m_grant_map_rw/p2m_grant_map_ro. So yeah, your variant is better. Thanks.
>
> I don't understand how grant mappings are related to this patch. What
> did you have in mind about them?

For some reason I assumed that following scenario would be possible:

Guest A grants access to own page to guest B. Guest B maps this page and
then shares it with OP-TEE.

My concern was that Guest B than can unmap granted page and release
grant, but it still be able to alter data on that page via OP-TEE.

But, looks I was mistaken there, because currently I'm checking p2m type to
be strictly p2m_ram_rw before sharing the page with OP-TEE. So mentioned
scenario is impossible in the first place.

-- 
Best regards,Volodymyr Babchuk
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel