search

linaro-swg / linux

Linux kernel source tree
Other
41 stars 79 forks source link

driver: tee: Handle NULL pointer indication from client #67

Closed cneveux closed 4 years ago

cneveux commented 5 years ago 
A client may need to present a NULL pointer in the argument passed to a
trusted application in the TEE. This is also a requirement in
GlobalPlatform Client API v1.0c (section 3.2.5 memory references), which
can be found at [1].

Only valid memory references could be passed prior to this patch. With this
patch a special case is added to recognize a NULL memory reference which
can be presented to a trusted application in the TEE. This patch contains
backwards compatible API change.

Link: [1] http://www.globalplatform.org/specificationsdevice.asp

Signed-off-by: Michael Whitfield michael.whitfield@nxp.com Signed-off-by: Cedric Neveux cedric.neveux@nxp.com

cneveux commented 5 years ago

yes, it could be. Here the ip.c is set by optee_client in case of error

jenswi-linaro commented 5 years ago

So it would possible to avoid driver modification if a valid (dummy) buffer was used instead? If that's the case, then I'd rather take that route.

cneveux commented 5 years ago

Without that, the GP Test fixed is resulting in a core system exception. This fix is part of the test.

cneveux commented 5 years ago

ok done. thanks

jenswi-linaro commented 5 years ago

Please address the comments too.

jenswi-linaro commented 5 years ago

Please rebase

cneveux commented 5 years ago

Ok done

jenswi-linaro commented 5 years ago

I made a few comments before. Please address those either by fixing it or by explaining why you think your way is preferable.

jforissier commented 5 years ago

The author and the first S-o-b have to be identical.

jbech-linaro commented 4 years ago

Not sure what state this is in, but since OP-TEE/optee_client#145 got merged. It seems like this needs to progress also.

jbech-linaro commented 4 years ago

Reviewed-by: Joakim Bech <joakim.bech@linaro.org> Tested-by: Joakim Bech <joakim.bech@linaro.org> (QEMU)

cneveux commented 4 years ago

Modification done following @jenswi-linaro recommandation. Problem is that I was not able to test it. (build is ok) I tried to build the qemu platform following with the build manifest on WSL environment but it's not building.

jbech-linaro commented 4 years ago

Problem is that I was not able to test it.

I can give this and the other patches a test run in my QEMU env. I have the original patches enabled. But I haven't tried with the changes done last week or so. Stay tuned ...

jbech-linaro commented 4 years ago

With this

xtest passes (including 1025 of course) and my Widevine test using this passes. Tested-by: Joakim Bech <joakim.bech@linaro.org> (QEMU)

jenswi-linaro commented 4 years ago

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

OK to merge to our local tree. I think we can let it stew there for a little before posting it on the mailing list. Just to make sure that everything is shipshape.

cneveux commented 4 years ago

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

OK to merge to our local tree. I think we can let it stew there for a little before posting it on the mailing list. Just to make sure that everything is shipshape.

Can I squash? Or do we wait @etienne-lms approval?

jenswi-linaro commented 4 years ago

Please go ahead and squash. This is so small it shouldn't be a problem to get an overview anyway.

jforissier commented 4 years ago

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

OK to merge to our local tree. I think we can let it stew there for a little before posting it on the mailing list. Just to make sure that everything is shipshape.

:stew: :yum:

Sounds good to me. @cneveux please also rebase/squash/apply tags to the optee_os PR, then I will merge things in the expected order (this, optee_os, optee_test).