Re: [PATCH 1/6] tee: qcomtee: Track the object invocation context
Harshal Dev via OP-TEE <[email protected]> Fri, 10 Jul 2026 12:07:00 +0530
| Newsgroups | org.trustedfirmware.lists.op-tee,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Amir, On 7/8/2026 11:31 AM, Amirreza Zarrabi wrote: > Hi Harshal, > > On 7/7/2026 4:11 PM, Harshal Dev wrote: >> QCOMTEE needs to distinguish between object invocations arriving from >> kernel clients and user-space clients in order to correctly marshal >> UBUF parameters and decide whether certain operations should be permitted. >> >> Add a kernel_ctx flag to struct qcomtee_object_invoke_context to track >> the context of object invocation. Objects invoked from the kernel-space >> are expected to have the MSB of their 64-bit object-id set to indicate a >> kernel context, whereas objects invoked from user-space should not set it. >> To ensure this, we restrict the object-id space of user-space invoked >> objects to 32-bits. This is in-line with QTEE expectation of 32-bit object >> ids. >> >> Signed-off-by: Amirreza Zarrabi <[email protected]> >> Signed-off-by: Harshal Dev <[email protected]> >> --- >> drivers/tee/qcomtee/call.c | 24 ++++++++++++++++++++++-- >> drivers/tee/qcomtee/qcomtee.h | 6 ++++++ >> drivers/tee/qcomtee/qcomtee_object.h | 8 ++++++-- >> drivers/tee/tee_core.c | 4 ++++ >> 4 files changed, 38 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/tee/qcomtee/call.c b/drivers/tee/qcomtee/call.c >> index 0efc5646242a..a74a54d67b06 100644 >> --- a/drivers/tee/qcomtee/call.c >> +++ b/drivers/tee/qcomtee/call.c >> @@ -397,11 +397,31 @@ static int qcomtee_object_invoke(struct tee_context *ctx, >> { >> struct qcomtee_context_data *ctxdata = ctx->data; >> struct qcomtee_object *object; >> + bool kernel_ctx = false; >> int i, ret, result; >> >> if (qcomtee_params_check(params, arg->num_params)) >> return -EINVAL; >> >> + /* Obtain the invocation context information from the MSB of the object >> + * `id` field. >> + */ >> + kernel_ctx = QCOMTEE_GET_CLIENT_CTX(arg->id); >> + /* User-space identifies a NULL object via a 32-bit TEE_OBJREF_NULL id, whereas >> + * the kernel uses as 64-bit object-id. Hence, we check for a NULL object by >> + * sign-extending the object-id to 64 bits. If user-space is indeed invoking a >> + * NULL object we must extend the object-id to 64-bits from here on so that >> + * QCOMTEE can recognize it. >> + */ >> + if (!kernel_ctx && ((s64)(s32)arg->id) == TEE_OBJREF_NULL) >> + arg->id = TEE_OBJREF_NULL; > > Does it need to be MSB -- why bit 63? the object ID supported by QTEE is 32-bit anyway. > Let's mask the upper 32-bit and do something like kernel_ctx = !!upper_32_bits(id). > What do you think? I agree. Instead of checking for kernel-ctx by right shifting, I can use this approach. QTEE objects invoked from user-space (including NULL object) will always have their object-id constrained to lower 32 bits anyway as per PR: https://github.com/quic/quic-teec/pull/27 And for kernel-space invoked QTEE objects, upper 32-bits would be set for NULL object, and 63rd bit expected to be set as well. > >> + >> + /* If the object being invoked is not NULL, drop the MSB from the `id` field to >> + * obtain the actual object-id. >> + */ >> + if (arg->id != TEE_OBJREF_NULL) >> + arg->id = QCOMTEE_SANITIZE_OBJ_ID(arg->id); >> + >> /* First, handle reserved operations: */ >> if (arg->op == QCOMTEE_MSG_OBJECT_OP_RELEASE) { >> del_qtee_object(arg->id, ctxdata); >> @@ -411,7 +431,7 @@ static int qcomtee_object_invoke(struct tee_context *ctx, >> >> /* Otherwise, invoke a QTEE object: */ >> struct qcomtee_object_invoke_ctx *oic __free(kfree) = >> - qcomtee_object_invoke_ctx_alloc(ctx); >> + qcomtee_object_invoke_ctx_alloc(ctx, kernel_ctx); >> if (!oic) >> return -ENOMEM; >> >> @@ -648,7 +668,7 @@ static void qcomtee_get_qtee_feature_list(struct tee_context *ctx, u32 id, >> int result; >> >> struct qcomtee_object_invoke_ctx *oic __free(kfree) = >> - qcomtee_object_invoke_ctx_alloc(ctx); >> + qcomtee_object_invoke_ctx_alloc(ctx, true); >> if (!oic) >> return; >> >> diff --git a/drivers/tee/qcomtee/qcomtee.h b/drivers/tee/qcomtee/qcomtee.h >> index f39bf63fd1c2..5d292a2ff83d 100644 >> --- a/drivers/tee/qcomtee/qcomtee.h >> +++ b/drivers/tee/qcomtee/qcomtee.h >> @@ -17,6 +17,12 @@ >> #define QCOMTEE_OBJREF_FLAG_USER BIT(1) >> #define QCOMTEE_OBJREF_FLAG_MEM BIT(2) >> >> +/* The MSB of the object_id field indicates whether the client is invoking the >> + * object from user context or kernel context. >> + */ >> +#define QCOMTEE_GET_CLIENT_CTX(x) (((x) >> 63) & 1U) >> +#define QCOMTEE_SANITIZE_OBJ_ID(x) ((x) & (BIT(63) - 1)) >> + >> /** >> * struct qcomtee - Main service struct. >> * @teedev: client device. >> diff --git a/drivers/tee/qcomtee/qcomtee_object.h b/drivers/tee/qcomtee/qcomtee_object.h >> index 8b4401ecad48..2528d07e4576 100644 >> --- a/drivers/tee/qcomtee/qcomtee_object.h >> +++ b/drivers/tee/qcomtee/qcomtee_object.h >> @@ -146,6 +146,7 @@ static inline int qcomtee_args_len(struct qcomtee_arg *args) >> * struct qcomtee_object_invoke_ctx - QTEE context for object invocation. >> * @ctx: TEE context for this invocation. >> * @flags: flags for the invocation context. >> + * @kernel_ctx: flag that indicates this context is owned by a kernel client. >> * @errno: error code for the invocation. >> * @object: current object invoked in this callback context. >> * @u: array of arguments for the current invocation (+1 for ending arg). >> @@ -158,6 +159,7 @@ static inline int qcomtee_args_len(struct qcomtee_arg *args) >> struct qcomtee_object_invoke_ctx { >> struct tee_context *ctx; >> unsigned long flags; >> + bool kernel_ctx; >> int errno; >> >> struct qcomtee_object *object; >> @@ -172,13 +174,15 @@ struct qcomtee_object_invoke_ctx { >> }; >> >> static inline struct qcomtee_object_invoke_ctx * >> -qcomtee_object_invoke_ctx_alloc(struct tee_context *ctx) >> +qcomtee_object_invoke_ctx_alloc(struct tee_context *ctx, bool kernel_ctx) >> { >> struct qcomtee_object_invoke_ctx *oic; >> >> oic = kzalloc_obj(*oic); >> - if (oic) >> + if (oic) { >> oic->ctx = ctx; >> + oic->kernel_ctx = kernel_ctx; >> + } >> return oic; >> } >> >> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c >> index ef9642d72672..7f986d7fb47f 100644 >> --- a/drivers/tee/tee_core.c >> +++ b/drivers/tee/tee_core.c >> @@ -706,6 +706,10 @@ static int tee_ioctl_object_invoke(struct tee_context *ctx, >> goto out; >> } >> >> + /* Userspace object-ids are restricted to 32-bits. */ >> + if (arg.id > U32_MAX) >> + return -EINVAL; >> + > > This change belongs to tee SS, move it to a separate commit with appropriate message. Ack. Regards, Harshal > >> rc = ctx->teedev->desc->ops->object_invoke_func(ctx, &arg, params); >> if (rc) >> goto out; >> > > Regards, > Amir >