Re: [PATCH v4 08/10] xen/arm: optee: add support for RPC commands

[Julien Grall <julien.grall@arm.com>]

On 20/03/2019 15:36, Volodymyr Babchuk wrote:
> 
> Hi Julien,

Hi Volodymyr,

> 
> Julien Grall writes:
> 
>>>        if ( shm_rpc->guest_page )
>>>            put_page(shm_rpc->guest_page);
>>> +
>>
>> Spurious change.
> 
> Good catch. Thank you.
> 
>>
>>> +        /*
>>> +         * TODO: With current implementation, OP-TEE will not issue
>>> +         * RPC to free this buffer. Guest and OP-TEE will be out of
>>> +         * sync: guest believes that it provided buffer to OP-TEE,
>>> +         * while OP-TEE thinks of opposite. Ideally, we need to
>>> +         * emulate RPC with OPTEE_MSG_RPC_CMD_SHM_FREE command.
>>> +         */
>> Can this condition happen if Xen runs out of memory?
> 
> Yes, this is one of reasons why translate_noncontig() might fail. It
> uses both xenheap to allocate data structure and domheap to allocate
> pagelists buffers. Any of those can fail, resulting in inconsistency.

That's not very ideal. This mean a well-behaving guest can get out of sync 
because of memory pressure (could be cause by a misbehaving guest on the 
platform). What will be the consequence for the guest? Can it continue safely?

This feature is under EXPERT mode so I would be ok to get as this in Xen. 
Although, I would like to hear Stefano's opinion here.

In any case, this should be fixed because we consider removing EXPERT mode and 
security support it.

> 
> You gave me idea to put gdprintk() with big fat warning there.

You clearly want to gprintk here because if this happen in non-debug build you 
want to know that something has gone really wrong as soon as possible.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel