The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
To: Harshal Dev <harshal.dev@oss.qualcomm.com>
Cc: Basant Kumar <basantk@qti.qualcomm.com>,
	Apurupa Pattapu <apurupa@qti.qualcomm.com>,
	Arun Kumar Neelakantam <aneelaka@qti.qualcomm.com>,
	op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	Konrad Dybcio <konradybcio@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Sumit Garg <sumit.garg@kernel.org>,
	jenswi@kernel.org
Subject: Re: [PATCH 1/6] tee: qcomtee: Track the object invocation context
Date: Wed, 15 Jul 2026 09:47:59 +1000	[thread overview]
Message-ID: <f917eb0f-9c9d-4f30-9c4f-ac3aeddac967@oss.qualcomm.com> (raw)
In-Reply-To: <fb1d4954-7c0f-4609-be40-b032876169b8@oss.qualcomm.com>

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.

- 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
>>
> 


  reply	other threads:[~2026-07-14 23:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  6:11 [PATCH 0/6] Add TEE based client driver for UEFI Secure Application Harshal Dev
2026-07-07  6:11 ` [PATCH 1/6] tee: qcomtee: Track the object invocation context Harshal Dev
2026-07-08  6:01   ` Amirreza Zarrabi
2026-07-10  6:37     ` Harshal Dev
2026-07-14 23:47       ` Amirreza Zarrabi [this message]
2026-07-16  6:58         ` Harshal Dev
2026-07-17  8:26           ` Sumit Garg
2026-07-20 10:18             ` Harshal Dev
2026-07-07  6:11 ` [PATCH 2/6] tee: Add kernel client object invoke helper Harshal Dev
2026-07-08  6:29   ` Amirreza Zarrabi
2026-07-10  6:52     ` Harshal Dev
2026-07-07  6:11 ` [PATCH 3/6] tee: qcomtee: Allow object invokes from kernel clients Harshal Dev
2026-07-07  6:11 ` [PATCH 4/6] tee: Export uuidv5 generation for TEE clients Harshal Dev
2026-07-07  6:11 ` [PATCH 5/6] tee: qcomtee: Add support for registering QTEE services on TEE bus Harshal Dev
2026-07-07  6:11 ` [PATCH 6/6] firmware: qcom: Add support for TEE based EFI-var client driver Harshal Dev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f917eb0f-9c9d-4f30-9c4f-ac3aeddac967@oss.qualcomm.com \
    --to=amirreza.zarrabi@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=aneelaka@qti.qualcomm.com \
    --cc=apurupa@qti.qualcomm.com \
    --cc=basantk@qti.qualcomm.com \
    --cc=harshal.dev@oss.qualcomm.com \
    --cc=jenswi@kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=op-tee@lists.trustedfirmware.org \
    --cc=sumit.garg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox