| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFA6WYNo2xJ8VncpwbHG65hTTshxa-SWaPnGCB2v3KCTGogJkQ@mail.gmail.com>
Date: Fri, 21 Jan 2022 19:07:00 +0530
From: Sumit Garg <sumit.garg@...aro.org>
To: Jerome Forissier <jerome@...issier.org>
Cc: Jens Wiklander <jens.wiklander@...aro.org>,
linux-kernel@...r.kernel.org, op-tee@...ts.trustedfirmware.org,
Herbert Xu <herbert@...dor.apana.org.au>,
Devaraj Rangasamy <Devaraj.Rangasamy@....com>,
Rijo Thomas <Rijo-john.Thomas@....com>,
David Howells <dhowells@...hat.com>,
Tyler Hicks <tyhicks@...ux.microsoft.com>,
Etienne Carriere <etienne.carriere@...aro.org>
Subject: Re: [PATCH v2 07/12] optee: use driver internal tee_contex for some rpc
On Fri, 21 Jan 2022 at 18:58, Jerome Forissier <jerome@...issier.org> wrote:
>
> On 1/21/22 13:54, Sumit Garg wrote:
> > + Jerome, Etienne
> >
> > On Fri, 14 Jan 2022 at 20:38, Jens Wiklander <jens.wiklander@...aro.org> wrote:
> >>
> >> Uses the new driver internal tee_context when allocating driver private
> >> shared memory. This decouples the shared memory object from its original
> >> tee_context. This is needed when the life time of such a memory
> >> allocation outlives the client tee_context.
> >>
> >> Signed-off-by: Jens Wiklander <jens.wiklander@...aro.org>
> >> ---
> >> drivers/tee/optee/ffa_abi.c | 16 ++++++++--------
> >> drivers/tee/optee/smc_abi.c | 7 ++++---
> >> 2 files changed, 12 insertions(+), 11 deletions(-)
> >>
> >
> > I guess with this patch we should no longer see issues [1] reported earlier.
>
> Correct. I have tested [1] again in QEMU with this whole series applied
> and the issue is gone.
>
Thanks for the confirmation.
Jens,
Can we have a Fixes tag for this and CC stable? I know it won't be
directly applicable to stable kernels but we can help with a backport.
-Sumit
> --
> Jerome
>
> >
> > FWIW,
> >
> > Reviewed-by: Sumit Garg <sumit.garg@...aro.org>
> >
> > [1] https://github.com/OP-TEE/optee_os/issues/1918
> >
> > -Sumit
> >
> >> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> >> index 88a028d4fb7b..5ec484b42432 100644
> >> --- a/drivers/tee/optee/ffa_abi.c
> >> +++ b/drivers/tee/optee/ffa_abi.c
> >> @@ -424,6 +424,7 @@ static struct tee_shm_pool *optee_ffa_shm_pool_alloc_pages(void)
> >> */
> >>
> >> static void handle_ffa_rpc_func_cmd_shm_alloc(struct tee_context *ctx,
> >> + struct optee *optee,
> >> struct optee_msg_arg *arg)
> >> {
> >> struct tee_shm *shm;
> >> @@ -439,7 +440,7 @@ static void handle_ffa_rpc_func_cmd_shm_alloc(struct tee_context *ctx,
> >> shm = optee_rpc_cmd_alloc_suppl(ctx, arg->params[0].u.value.b);
> >> break;
> >> case OPTEE_RPC_SHM_TYPE_KERNEL:
> >> - shm = tee_shm_alloc_priv_kernel_buf(ctx,
> >> + shm = tee_shm_alloc_priv_kernel_buf(optee->ctx,
> >> arg->params[0].u.value.b);
> >> break;
> >> default:
> >> @@ -493,14 +494,13 @@ static void handle_ffa_rpc_func_cmd_shm_free(struct tee_context *ctx,
> >> }
> >>
> >> static void handle_ffa_rpc_func_cmd(struct tee_context *ctx,
> >> + struct optee *optee,
> >> struct optee_msg_arg *arg)
> >> {
> >> - struct optee *optee = tee_get_drvdata(ctx->teedev);
> >> -
> >> arg->ret_origin = TEEC_ORIGIN_COMMS;
> >> switch (arg->cmd) {
> >> case OPTEE_RPC_CMD_SHM_ALLOC:
> >> - handle_ffa_rpc_func_cmd_shm_alloc(ctx, arg);
> >> + handle_ffa_rpc_func_cmd_shm_alloc(ctx, optee, arg);
> >> break;
> >> case OPTEE_RPC_CMD_SHM_FREE:
> >> handle_ffa_rpc_func_cmd_shm_free(ctx, optee, arg);
> >> @@ -510,12 +510,12 @@ static void handle_ffa_rpc_func_cmd(struct tee_context *ctx,
> >> }
> >> }
> >>
> >> -static void optee_handle_ffa_rpc(struct tee_context *ctx, u32 cmd,
> >> - struct optee_msg_arg *arg)
> >> +static void optee_handle_ffa_rpc(struct tee_context *ctx, struct optee *optee,
> >> + u32 cmd, struct optee_msg_arg *arg)
> >> {
> >> switch (cmd) {
> >> case OPTEE_FFA_YIELDING_CALL_RETURN_RPC_CMD:
> >> - handle_ffa_rpc_func_cmd(ctx, arg);
> >> + handle_ffa_rpc_func_cmd(ctx, optee, arg);
> >> break;
> >> case OPTEE_FFA_YIELDING_CALL_RETURN_INTERRUPT:
> >> /* Interrupt delivered by now */
> >> @@ -582,7 +582,7 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
> >> * above.
> >> */
> >> cond_resched();
> >> - optee_handle_ffa_rpc(ctx, data->data1, rpc_arg);
> >> + optee_handle_ffa_rpc(ctx, optee, data->data1, rpc_arg);
> >> cmd = OPTEE_FFA_YIELDING_CALL_RESUME;
> >> data->data0 = cmd;
> >> data->data1 = 0;
> >> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> >> index 1dbb13b08381..f2ef76451443 100644
> >> --- a/drivers/tee/optee/smc_abi.c
> >> +++ b/drivers/tee/optee/smc_abi.c
> >> @@ -621,6 +621,7 @@ static void handle_rpc_func_cmd_shm_free(struct tee_context *ctx,
> >> }
> >>
> >> static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx,
> >> + struct optee *optee,
> >> struct optee_msg_arg *arg,
> >> struct optee_call_ctx *call_ctx)
> >> {
> >> @@ -650,7 +651,7 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx,
> >> shm = optee_rpc_cmd_alloc_suppl(ctx, sz);
> >> break;
> >> case OPTEE_RPC_SHM_TYPE_KERNEL:
> >> - shm = tee_shm_alloc_priv_kernel_buf(ctx, sz);
> >> + shm = tee_shm_alloc_priv_kernel_buf(optee->ctx, sz);
> >> break;
> >> default:
> >> arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> >> @@ -746,7 +747,7 @@ static void handle_rpc_func_cmd(struct tee_context *ctx, struct optee *optee,
> >> switch (arg->cmd) {
> >> case OPTEE_RPC_CMD_SHM_ALLOC:
> >> free_pages_list(call_ctx);
> >> - handle_rpc_func_cmd_shm_alloc(ctx, arg, call_ctx);
> >> + handle_rpc_func_cmd_shm_alloc(ctx, optee, arg, call_ctx);
> >> break;
> >> case OPTEE_RPC_CMD_SHM_FREE:
> >> handle_rpc_func_cmd_shm_free(ctx, arg);
> >> @@ -775,7 +776,7 @@ static void optee_handle_rpc(struct tee_context *ctx,
> >>
> >> switch (OPTEE_SMC_RETURN_GET_RPC_FUNC(param->a0)) {
> >> case OPTEE_SMC_RPC_FUNC_ALLOC:
> >> - shm = tee_shm_alloc_priv_kernel_buf(ctx, param->a1);
> >> + shm = tee_shm_alloc_priv_kernel_buf(optee->ctx, param->a1);
> >> if (!IS_ERR(shm) && !tee_shm_get_pa(shm, 0, &pa)) {
> >> reg_pair_from_64(¶m->a1, ¶m->a2, pa);
> >> reg_pair_from_64(¶m->a4, ¶m->a5,
> >> --
> >> 2.31.1
> >>
Powered by blists - more mailing lists