Re: [Xen-devel] [PATCH v2 4/6] xen/arm: optee: handle shared buffer translation error

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi Julien,

Julien Grall writes:

> Hi,
>
> On 18/09/2019 19:51, Volodymyr Babchuk wrote:
>> +/* Handles return from Xen-issued RPC */
>> +static void handle_xen_rpc_return(struct optee_domain *ctx,
>> +                                  struct cpu_user_regs *regs,
>> +                                  struct optee_std_call *call,
>> +                                  struct shm_rpc *shm_rpc)
>> +{
>> +    call->state = OPTEE_CALL_NORMAL;
>> +
>> +    /*
>> +     * Right now we have only one reason to be there - we asked guest
>> +     * to free shared buffer and it did it. Now we can tell OP-TEE
>> +     * that buffer allocation failed. We are not storing exact command
>> +     * type, only type of RPC return. So, this is the only check we
>> +     * can perform there.
>> +     */
>> +    ASSERT(call->rpc_op == OPTEE_SMC_RPC_FUNC_CMD);
>
> As I pointed out in v1, ASSERT() is probably the less desirable
> solution here as this is an error path.
Looks like I misunderstood you :(

> Can you explain why you chose that over the 3 solutions I suggested?
I think I need some clarification there. ASSERT() is adopted way to tell
about invariant. Clearly, this is programming error if ASSERT() fails.

But I understand, that ASSERT() is available only in debug build. So, in
release it will never fire. As this is very unlikely error path, bug
there can live forever. Okay, so ASSERT() is the least desirable way.

Warning is not enough, as we are already in incorrect state.

So, best way is to crash guest, it this correct? I'll do this in the
next version then.

-- 
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel