Re: [PATCH 1/6] tee: qcomtee: Track the object invocation context
Amirreza Zarrabi via OP-TEE <[email protected]> Wed, 15 Jul 2026 09:47:59 +1000
| Newsgroups | org.trustedfirmware.lists.op-tee,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Harshal, On 7/10/2026 4:37 PM, Harshal Dev wrote: > 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. > Re-thinking this, I'm a bit concerned about overloading id while keeping it as a u64, and then filtering its upper 32 bits in tee_ioctl_object_invoke(). It feels unintuitive, as it naturally raises the question: if only the lower 32 bits are meaningful, why is id a `u64` in the first place? What if we make the caller origin explicit instead, for example: enum tee_object_invoke_origin { TEE_OBJECT_INVOKE_USERSPACE, TEE_OBJECT_INVOKE_KERNEL, }; and then pass it through the backend op: int (*object_invoke_func)(struct tee_context *ctx, struct tee_ioctl_object_invoke_arg *arg, struct tee_param *param, enum tee_object_invoke_origin origin); It seems more straightforward and avoids encoding caller-origin metadata into id. If you are OK, let's do this. - Amir >> >>> + >>> + /* 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 >> >