search

linaro-swg / linux

Linux kernel source tree
Other
41 stars 79 forks source link

tee/optee/call.c, optee_open_session() is not using arg->clnt_uuid #44

Open yyyledu opened 7 years ago

yyyledu commented 7 years ago

In call.c, optee_open_session(), the destination uuid in arg->uuid is being copied twice instead of copying the arg->clnt_uuid field.

memcpy(&msg_arg->params[0].u.value, arg->uuid, sizeof(arg->uuid));
memcpy(&msg_arg->params[1].u.value, arg->uuid, sizeof(arg->clnt_uuid));

Note, in optee_client/libteec/src/tee_client_api.c TEEC_OpenSession() clnt_uuid is not being initialized.

jforissier commented 7 years ago

In call.c, optee_open_session(), the destination uuid in arg->uuid is being copied twice instead of copying the arg->clnt_uuid field.

Indeed, looks like a bug to me.

Note, in optee_client/libteec/src/tee_client_api.c TEEC_OpenSession() clnt_uuid is not being initialized.

Well it is set to zero which is OK at least for the TEEC_LOGIN_PUBLIC connection method. Now I'm a bit confused about this clnt_id parameter, it looks like it was meant to be used with connection methods != TEEC_LOGIN_PUBLIC (see here: https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/tee/entry_std.c#L200-L206). But,TEEC_OpenSession() does not take a UUID, it takes an opaque (implementation-dependent) pointer. Since our implementation of TEEC_OpenSession() does not do anything to encode the clnt_id value from the const void *connection_data parameter, I think it should reject connection_method values other than TEEC_LOGIN_PUBLIC.

jenswi-linaro commented 7 years ago

Yes, it's a bug.

I agree that we should reject connection_method values other than TEEC_LOGIN_PUBLIC. But the question is when and where. Always In the framework or only if TEE_GEN_CAP_GP is set or only inside the OP-TEE driver?