On Thu, 11 May 2023 at 08:03, Sumit Garg sumit.garg@linaro.org wrote:
(snip)
int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg, @@ -408,12 +412,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg, struct optee_msg_arg *msg_arg; struct optee_session *sess; struct tee_shm *shm;
bool system_thread; u_int offs; int rc; /* Check that the session is valid */ mutex_lock(&ctxdata->mutex); sess = find_session(ctxdata, arg->session);if (sess)This check is redundant if we move the assignment below...
Here we change the sesssion attribute while the mutex is locked, in case some equivalent call with that session is issued. Below we return to caller once mutex is unlocked. I think it is the safer behavior. What do you think?
Aren't we only reading session attribute in order to capture value in a local variable: system_thread? I don't think that it would require a mutex.
optee_system_session() sets session::use_sys_thread with mutex locked hence I think we should get the attribute with the mutex locked. See "[PATCH v6 3/4] tee: optee: support tracking system threads".
Etienne
-Sumit
Best regards, Etienne
system_thread = sess->use_sys_thread; mutex_unlock(&ctxdata->mutex); if (!sess) return -EINVAL;...here as: system_thread = sess->use_sys_thread; (snip)