Hi Julien,
Julien Grall writes:
Hi Volodymyr,
On 18/01/2019 16:05, Volodymyr Babchuk wrote:
Julien Grall writes:
If you happen to make MAX_STD_CALLS dynamic, then this should also be dynamic.
Of course.
[...]
>> +static void handle_rpc_func_alloc(struct optee_domain *ctx, >> + struct cpu_user_regs *regs) >> +{ >> + paddr_t ptr = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); >> + >> + if ( ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) ) >> + gprintk(XENLOG_WARNING, "Domain returned invalid RPC command buffer\n"); > > Should not you bail-out in that case? Also, I would turn it to a gdprintk. OP-TEE does own checks and that check will fail also. Then OP-TEE will issue request to free this SHM.
I think it is better if we go on the safe-side. I.e if we know there would be an error (like here), then you need to return an error in from Xen rather than calling OP-TEE. Otherwise, you may end up to nasty problem.
Actually, I don't see how I can do this. This is an RPC response. I can't return error to RPC response. All I can do is to mangle RPC response in a such way, that OP-TEE surely will treat it as an error and act accordingly.
I am not sure to understand what you mean here. Surely if the address is not aligned, then OP-TEE will return an error too, right? So can't you emulate OP-TEE behavior in that case?
No, OP-TEE will not return an error. OP-TEE will handle it as an error.
Will it?
Yes. It checks for page alignment in the same way, as Xen does.
Looking at the code again, you will pass either 0 (when not able to translate the address) or page_to_maddr(...) value. So the value will get truncated by Xen with just a warning.
Is it the expected behavior?
No, this is a bug and I'll fix it.
For OP-TEE any RPC looks like a function call. So it excepts some return value - buffer pointer in this case. If it gets invalid pointer, it issues another RPC to free this buffer and then propagates error back to caller.
So, if I'll return error back to the guest (or rather issue RPC to free the buffer), OP-TEE will be still blocked, waiting for a pointer from the guest. Of course I can go further end emulate a new buffer allocation RPC but from Xen side in a hope that guest will provide valid buffer this time. But, what if not? And why I should decide instead of OP-TEE?
My main concern here is you do a print a warning and continue like nothing happen. As a reviewer, this is worrying because it is a call for weird behavior to happen in Xen. We don't want that.
Yep, I made a mistake there. I need to pass invalid value to OP-TEE to make sure that it will not mistake it with semi-valid address.
Looking at the code, why can't you pass 0 as you do if you fail to pin the pages?
This is exactly what I'm going to do.