search

linaro-swg / linux

Linux kernel source tree
Other
41 stars 79 forks source link

[NEW SHM] tee: optee: remove registered shm size check #53

Closed lorc closed 6 years ago

lorc commented 6 years ago

We don't need to return error if output shm size is larger than allocated buffer. This is pefrectly fine. In this way TEE or TA reports that it needs larger buffer (as described in TEE Client API Specification v1.0, section 3.2.5.).

Signed-off-by: Volodymyr Babchuk vlad.babchuk@gmail.com

This patch fixes issue found at https://github.com/OP-TEE/optee_os/pull/1830#issuecomment-334801115

jbech-linaro commented 6 years ago

I had a look at the spec and I think you are right. But the spec also recommends to return TEEC_ERROR_SHORT_BUFFER. Having that said it is not kernel that should return that I think, but we should double check that we indeed are returning that on secure side (and also update the size accordingly).

The only comment otherwise is the commit header, can we shorten it to: optee: remove registered shm size check?

Other than that: Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

jforissier commented 6 years ago

Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)

Tested with dynamic SHM on HiKey (and without SDP due to an issue with kernel 4.12). All tests pass, including GP 8019/8024/8043. Thanks for the quick fix!

jforissier commented 6 years ago

@jbech-linaro

But the spec also recommends to return TEEC_ERROR_SHORT_BUFFER. [...] we should double check that we indeed are returning that [...]

It's exactly the purpose of GP 8019/8024/8043 ;-)

[...] (and also update the size accordingly).

That, however, is not tested by the above tests.

lorc commented 6 years ago

I'm looking how to return TEEC_ERROR_SHORT_BUFFER from the kernel in case if OP-TEE/TA missed that. Looks like I need to rework optee_from_msg_param(), so it will be a larger patch.

lorc commented 6 years ago

I've changed commit message for original patch and pushed second patch, that returns TEEC_ERROR_SHORT_BUFFER in case if OP-TEE missed to do so.

jforissier commented 6 years ago

I'm not sure I like the more complex fix (not sure it is necessary/useful). Anyway, still working good here:

Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)

lorc commented 6 years ago

@jforissier, actually, I am also not sure that second solution is better. I prefer simpler one. Anyway, thank you for testing.

lorc commented 6 years ago

Okay. So I've pushed the first version back (with altered caption, as @jbech-linaro suggested). Also I added R-b and T-b tags, as they were for that version.

jenswi-linaro commented 6 years ago

Looks good