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

Re: [Xen-devel] [PATCH v3 07/11] optee: add support for RPC SHM buffers



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

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.

Looking at the code, why can't you pass 0 as you do if you fail to pin the 
pages?

Cheers,

--
Julien Grall

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

 


Rackspace

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