Re: [PATCH 1/6] tee: qcomtee: Track the object invocation context

[Harshal Dev <harshal.dev@oss.qualcomm.com>]

Hi Amir,

On 7/15/2026 5:17 AM, Amirreza Zarrabi wrote:
> 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 <amirreza.zarrabi@oss.qualcomm.com>
>>>> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
>>>> ---
>>>>  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.
>

Honestly, this approach makes everything a lot more easier, since we don't need complex
bit manipulation in back-end to filter the upper 32 bits and handle the case of
TEE_OBJREF_NULL. I can also then drop the PR: https://github.com/quic/quic-teec/pull/27

I am fine with updating this callback and calling it with TEE_OBJECT_INVOKE_USERSPACE from
https://elixir.bootlin.com/linux/v7.1/source/drivers/tee/tee_core.c#L704
and TEE_OBJECT_INVOKE_KERNEL from the newly introduced tee_client_object_invoke_func().

But is there any special reason for choosing an enum over a boolean value here? Are we planning
to extend the enum later to introduce any other context information?

I am fine with this approach. I hope the OP-TEE maintainers @Sumit are fine as well with the
new parameter addition in the callback.

Regards,
Harshal 
> - 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
>>>
>>
>