* [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
[not found] <=?utf-8?q?=3C20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-?= =?utf-8?q?0-7f457073282d=40oss=2Equalcomm=2Ecom=3E?=>
@ 2025-03-28 2:47 ` Amirreza Zarrabi
0 siblings, 0 replies; 9+ messages in thread
From: Amirreza Zarrabi @ 2025-03-28 2:47 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 5516 bytes --]
For drivers that can transfer data to the TEE without using shared
memory from client, it is necessary to receive the user address
directly, bypassing any processing by the TEE subsystem. Introduce
TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
userspace buffers.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
include/linux/tee_drv.h | 6 ++++++
include/uapi/linux/tee.h | 22 ++++++++++++++++------
3 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 22cc7d624b0c..bc862a11d437 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
params[n].u.value.b = ip.b;
params[n].u.value.c = ip.c;
break;
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
+ params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
+ params[n].u.ubuf.size = ip.b;
+
+ if (!access_ok(params[n].u.ubuf.uaddr,
+ params[n].u.ubuf.size))
+ return -EFAULT;
+
+ break;
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
@@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
put_user(p->u.value.c, &up->c))
return -EFAULT;
break;
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
+ if (put_user((u64)p->u.ubuf.size, &up->b))
+ return -EFAULT;
+ break;
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
if (put_user((u64)p->u.memref.size, &up->b))
@@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
ip.b = p->u.value.b;
ip.c = p->u.value.c;
break;
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
+ ip.a = (u64)p->u.ubuf.uaddr;
+ ip.b = p->u.ubuf.size;
+ ip.c = 0;
+ break;
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
@@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
p->u.value.b = ip.b;
p->u.value.c = ip.c;
break;
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
+ case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
+ p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
+ p->u.ubuf.size = ip.b;
+
+ if (!access_ok(params[n].u.ubuf.uaddr,
+ params[n].u.ubuf.size))
+ return -EFAULT;
+
+ break;
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
/*
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index ce23fd42c5d4..d773f91c6bdd 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -82,6 +82,11 @@ struct tee_param_memref {
struct tee_shm *shm;
};
+struct tee_param_ubuf {
+ void * __user uaddr;
+ size_t size;
+};
+
struct tee_param_value {
u64 a;
u64 b;
@@ -92,6 +97,7 @@ struct tee_param {
u64 attr;
union {
struct tee_param_memref memref;
+ struct tee_param_ubuf ubuf;
struct tee_param_value value;
} u;
};
diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
index d0430bee8292..3e9b1ec5dfde 100644
--- a/include/uapi/linux/tee.h
+++ b/include/uapi/linux/tee.h
@@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
#define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
#define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
+/*
+ * These defines userspace buffer parameters.
+ */
+#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
+#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
+#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
+
/*
* Mask for the type part of the attribute, leaves room for more types
*/
@@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
/**
* struct tee_ioctl_param - parameter
* @attr: attributes
- * @a: if a memref, offset into the shared memory object, else a value parameter
- * @b: if a memref, size of the buffer, else a value parameter
+ * @a: if a memref, offset into the shared memory object,
+ * else if a ubuf, address of the user buffer,
+ * else a value parameter
+ * @b: if a memref or ubuf, size of the buffer, else a value parameter
* @c: if a memref, shared memory identifier, else a value parameter
*
- * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
- * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
- * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
- * indicates that none of the members are used.
+ * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
+ * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
+ * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
+ * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
+ * are used.
*
* Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
* identifier representing the shared memory object. A memref can reference
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
[not found] <=?utf-8?q?=3C20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-?= =?utf-8?q?3-7f457073282d=40oss=2Equalcomm=2Ecom=3E?=>
@ 2025-03-29 4:58 ` kernel test robot
2025-04-08 12:19 ` Jens Wiklander
1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-03-29 4:58 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 6398 bytes --]
Hi Amirreza,
kernel test robot noticed the following build warnings:
[auto build test WARNING on db8da9da41bced445077925f8a886c776a47440c]
url: https://github.com/intel-lab-lkp/linux/commits/Amirreza-Zarrabi/tee-allow-a-driver-to-allocate-a-tee_device-without-a-pool/20250328-104950
base: db8da9da41bced445077925f8a886c776a47440c
patch link: https://lore.kernel.org/r/20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-3-7f457073282d%40oss.qualcomm.com
patch subject: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
config: x86_64-randconfig-122-20250329 (https://download.01.org/0day-ci/archive/20250329/202503291204.imMRd3l7-lkp(a)intel.com/config)
compiler: clang version 20.1.1 (https://github.com/llvm/llvm-project 424c2d9b7e4de40d0804dd374721e6411c27d1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250329/202503291204.imMRd3l7-lkp(a)intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503291204.imMRd3l7-lkp(a)intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/tee/tee_core.c:410:48: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *[noderef] uaddr @@ got void [noderef] __user * @@
drivers/tee/tee_core.c:410:48: sparse: expected void *[noderef] uaddr
drivers/tee/tee_core.c:410:48: sparse: got void [noderef] __user *
>> drivers/tee/tee_core.c:413:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user *ptr @@ got void *[noderef] uaddr @@
drivers/tee/tee_core.c:413:30: sparse: expected void const [noderef] __user *ptr
drivers/tee/tee_core.c:413:30: sparse: got void *[noderef] uaddr
drivers/tee/tee_core.c:802:41: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *[noderef] uaddr @@ got void [noderef] __user * @@
drivers/tee/tee_core.c:802:41: sparse: expected void *[noderef] uaddr
drivers/tee/tee_core.c:802:41: sparse: got void [noderef] __user *
drivers/tee/tee_core.c:805:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user *ptr @@ got void *[noderef] uaddr @@
drivers/tee/tee_core.c:805:30: sparse: expected void const [noderef] __user *ptr
drivers/tee/tee_core.c:805:30: sparse: got void *[noderef] uaddr
>> drivers/tee/tee_core.c:413:30: sparse: sparse: dereference of noderef expression
>> drivers/tee/tee_core.c:413:30: sparse: sparse: dereference of noderef expression
drivers/tee/tee_core.c:694:37: sparse: sparse: dereference of noderef expression
drivers/tee/tee_core.c:805:30: sparse: sparse: dereference of noderef expression
drivers/tee/tee_core.c:805:30: sparse: sparse: dereference of noderef expression
vim +410 drivers/tee/tee_core.c
378
379 static int params_from_user(struct tee_context *ctx, struct tee_param *params,
380 size_t num_params,
381 struct tee_ioctl_param __user *uparams)
382 {
383 size_t n;
384
385 for (n = 0; n < num_params; n++) {
386 struct tee_shm *shm;
387 struct tee_ioctl_param ip;
388
389 if (copy_from_user(&ip, uparams + n, sizeof(ip)))
390 return -EFAULT;
391
392 /* All unused attribute bits has to be zero */
393 if (ip.attr & ~TEE_IOCTL_PARAM_ATTR_MASK)
394 return -EINVAL;
395
396 params[n].attr = ip.attr;
397 switch (ip.attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK) {
398 case TEE_IOCTL_PARAM_ATTR_TYPE_NONE:
399 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT:
400 break;
401 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT:
402 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT:
403 params[n].u.value.a = ip.a;
404 params[n].u.value.b = ip.b;
405 params[n].u.value.c = ip.c;
406 break;
407 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
408 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
409 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> 410 params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
411 params[n].u.ubuf.size = ip.b;
412
> 413 if (!access_ok(params[n].u.ubuf.uaddr,
414 params[n].u.ubuf.size))
415 return -EFAULT;
416
417 break;
418 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
419 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
420 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
421 /*
422 * If a NULL pointer is passed to a TA in the TEE,
423 * the ip.c IOCTL parameters is set to TEE_MEMREF_NULL
424 * indicating a NULL memory reference.
425 */
426 if (ip.c != TEE_MEMREF_NULL) {
427 /*
428 * If we fail to get a pointer to a shared
429 * memory object (and increase the ref count)
430 * from an identifier we return an error. All
431 * pointers that has been added in params have
432 * an increased ref count. It's the callers
433 * responibility to do tee_shm_put() on all
434 * resolved pointers.
435 */
436 shm = tee_shm_get_from_id(ctx, ip.c);
437 if (IS_ERR(shm))
438 return PTR_ERR(shm);
439
440 /*
441 * Ensure offset + size does not overflow
442 * offset and does not overflow the size of
443 * the referred shared memory object.
444 */
445 if ((ip.a + ip.b) < ip.a ||
446 (ip.a + ip.b) > shm->size) {
447 tee_shm_put(shm);
448 return -EINVAL;
449 }
450 } else if (ctx->cap_memref_null) {
451 /* Pass NULL pointer to OP-TEE */
452 shm = NULL;
453 } else {
454 return -EINVAL;
455 }
456
457 params[n].u.memref.shm_offs = ip.a;
458 params[n].u.memref.size = ip.b;
459 params[n].u.memref.shm = shm;
460 break;
461 default:
462 /* Unknown attribute */
463 return -EINVAL;
464 }
465 }
466 return 0;
467 }
468
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
[not found] <=?utf-8?q?=3C20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-?= =?utf-8?q?3-7f457073282d=40oss=2Equalcomm=2Ecom=3E?=>
2025-03-29 4:58 ` kernel test robot
@ 2025-04-08 12:19 ` Jens Wiklander
1 sibling, 0 replies; 9+ messages in thread
From: Jens Wiklander @ 2025-04-08 12:19 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 7136 bytes --]
Hi Amirreza,
On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> For drivers that can transfer data to the TEE without using shared
> memory from client, it is necessary to receive the user address
> directly, bypassing any processing by the TEE subsystem. Introduce
> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
> userspace buffers.
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/tee_drv.h | 6 ++++++
> include/uapi/linux/tee.h | 22 ++++++++++++++++------
> 3 files changed, 55 insertions(+), 6 deletions(-)
Is this patch needed now that the QCOMTEE driver supports shared
memory? I prefer keeping changes to the ABI to a minimum.
Cheers,
Jens
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 22cc7d624b0c..bc862a11d437 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
> params[n].u.value.b = ip.b;
> params[n].u.value.c = ip.c;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> + params[n].u.ubuf.size = ip.b;
> +
> + if (!access_ok(params[n].u.ubuf.uaddr,
> + params[n].u.ubuf.size))
> + return -EFAULT;
> +
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
> put_user(p->u.value.c, &up->c))
> return -EFAULT;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + if (put_user((u64)p->u.ubuf.size, &up->b))
> + return -EFAULT;
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> if (put_user((u64)p->u.memref.size, &up->b))
> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
> ip.b = p->u.value.b;
> ip.c = p->u.value.c;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + ip.a = (u64)p->u.ubuf.uaddr;
> + ip.b = p->u.ubuf.size;
> + ip.c = 0;
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
> p->u.value.b = ip.b;
> p->u.value.c = ip.c;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> + p->u.ubuf.size = ip.b;
> +
> + if (!access_ok(params[n].u.ubuf.uaddr,
> + params[n].u.ubuf.size))
> + return -EFAULT;
> +
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> /*
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index ce23fd42c5d4..d773f91c6bdd 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -82,6 +82,11 @@ struct tee_param_memref {
> struct tee_shm *shm;
> };
>
> +struct tee_param_ubuf {
> + void * __user uaddr;
> + size_t size;
> +};
> +
> struct tee_param_value {
> u64 a;
> u64 b;
> @@ -92,6 +97,7 @@ struct tee_param {
> u64 attr;
> union {
> struct tee_param_memref memref;
> + struct tee_param_ubuf ubuf;
> struct tee_param_value value;
> } u;
> };
> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> index d0430bee8292..3e9b1ec5dfde 100644
> --- a/include/uapi/linux/tee.h
> +++ b/include/uapi/linux/tee.h
> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
>
> +/*
> + * These defines userspace buffer parameters.
> + */
> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
> +
> /*
> * Mask for the type part of the attribute, leaves room for more types
> */
> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
> /**
> * struct tee_ioctl_param - parameter
> * @attr: attributes
> - * @a: if a memref, offset into the shared memory object, else a value parameter
> - * @b: if a memref, size of the buffer, else a value parameter
> + * @a: if a memref, offset into the shared memory object,
> + * else if a ubuf, address of the user buffer,
> + * else a value parameter
> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
> * @c: if a memref, shared memory identifier, else a value parameter
> *
> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
> - * indicates that none of the members are used.
> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
> + * are used.
> *
> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
> * identifier representing the shared memory object. A memref can reference
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
[not found] < <CAHUa44GRBiRr6CsFWxJhyzf1cRSEP66m5K7uFntOv3oYWTHWgQ@mail.gmail.com>
@ 2025-04-09 0:28 ` Amirreza Zarrabi
2025-04-09 6:41 ` Jens Wiklander
0 siblings, 1 reply; 9+ messages in thread
From: Amirreza Zarrabi @ 2025-04-09 0:28 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 7727 bytes --]
Hi jens,
On 4/8/2025 10:19 PM, Jens Wiklander wrote:
> Hi Amirreza,
>
> On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>> For drivers that can transfer data to the TEE without using shared
>> memory from client, it is necessary to receive the user address
>> directly, bypassing any processing by the TEE subsystem. Introduce
>> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
>> userspace buffers.
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
>> include/linux/tee_drv.h | 6 ++++++
>> include/uapi/linux/tee.h | 22 ++++++++++++++++------
>> 3 files changed, 55 insertions(+), 6 deletions(-)
> Is this patch needed now that the QCOMTEE driver supports shared
> memory? I prefer keeping changes to the ABI to a minimum.
>
> Cheers,
> Jens
Unfortunately, this is still required. QTEE supports two types of data transfer:
(1) using UBUF and (2) memory objects. Even with memory object support, some APIs still
expect to receive data using UBUF. For instance, to load a TA, QTEE offers two interfaces:
one where the TA binary is in UBUF and another where the TA binary is in a memory object.
Best Regards,
Amir
>> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
>> index 22cc7d624b0c..bc862a11d437 100644
>> --- a/drivers/tee/tee_core.c
>> +++ b/drivers/tee/tee_core.c
>> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
>> params[n].u.value.b = ip.b;
>> params[n].u.value.c = ip.c;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
>> + params[n].u.ubuf.size = ip.b;
>> +
>> + if (!access_ok(params[n].u.ubuf.uaddr,
>> + params[n].u.ubuf.size))
>> + return -EFAULT;
>> +
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
>> put_user(p->u.value.c, &up->c))
>> return -EFAULT;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + if (put_user((u64)p->u.ubuf.size, &up->b))
>> + return -EFAULT;
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> if (put_user((u64)p->u.memref.size, &up->b))
>> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
>> ip.b = p->u.value.b;
>> ip.c = p->u.value.c;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + ip.a = (u64)p->u.ubuf.uaddr;
>> + ip.b = p->u.ubuf.size;
>> + ip.c = 0;
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
>> p->u.value.b = ip.b;
>> p->u.value.c = ip.c;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
>> + p->u.ubuf.size = ip.b;
>> +
>> + if (!access_ok(params[n].u.ubuf.uaddr,
>> + params[n].u.ubuf.size))
>> + return -EFAULT;
>> +
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> /*
>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
>> index ce23fd42c5d4..d773f91c6bdd 100644
>> --- a/include/linux/tee_drv.h
>> +++ b/include/linux/tee_drv.h
>> @@ -82,6 +82,11 @@ struct tee_param_memref {
>> struct tee_shm *shm;
>> };
>>
>> +struct tee_param_ubuf {
>> + void * __user uaddr;
>> + size_t size;
>> +};
>> +
>> struct tee_param_value {
>> u64 a;
>> u64 b;
>> @@ -92,6 +97,7 @@ struct tee_param {
>> u64 attr;
>> union {
>> struct tee_param_memref memref;
>> + struct tee_param_ubuf ubuf;
>> struct tee_param_value value;
>> } u;
>> };
>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>> index d0430bee8292..3e9b1ec5dfde 100644
>> --- a/include/uapi/linux/tee.h
>> +++ b/include/uapi/linux/tee.h
>> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
>> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
>> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
>>
>> +/*
>> + * These defines userspace buffer parameters.
>> + */
>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
>> +
>> /*
>> * Mask for the type part of the attribute, leaves room for more types
>> */
>> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
>> /**
>> * struct tee_ioctl_param - parameter
>> * @attr: attributes
>> - * @a: if a memref, offset into the shared memory object, else a value parameter
>> - * @b: if a memref, size of the buffer, else a value parameter
>> + * @a: if a memref, offset into the shared memory object,
>> + * else if a ubuf, address of the user buffer,
>> + * else a value parameter
>> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
>> * @c: if a memref, shared memory identifier, else a value parameter
>> *
>> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
>> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
>> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
>> - * indicates that none of the members are used.
>> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
>> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
>> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
>> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
>> + * are used.
>> *
>> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
>> * identifier representing the shared memory object. A memref can reference
>>
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
2025-04-09 0:28 ` Amirreza Zarrabi
@ 2025-04-09 6:41 ` Jens Wiklander
0 siblings, 0 replies; 9+ messages in thread
From: Jens Wiklander @ 2025-04-09 6:41 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 7970 bytes --]
Hi Amirreza,
On Wed, Apr 9, 2025 at 2:28 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> Hi jens,
>
> On 4/8/2025 10:19 PM, Jens Wiklander wrote:
>
> Hi Amirreza,
>
> On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> For drivers that can transfer data to the TEE without using shared
> memory from client, it is necessary to receive the user address
> directly, bypassing any processing by the TEE subsystem. Introduce
> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
> userspace buffers.
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/tee_drv.h | 6 ++++++
> include/uapi/linux/tee.h | 22 ++++++++++++++++------
> 3 files changed, 55 insertions(+), 6 deletions(-)
>
> Is this patch needed now that the QCOMTEE driver supports shared
> memory? I prefer keeping changes to the ABI to a minimum.
>
> Cheers,
> Jens
>
> Unfortunately, this is still required. QTEE supports two types of data transfer:
> (1) using UBUF and (2) memory objects. Even with memory object support, some APIs still
> expect to receive data using UBUF. For instance, to load a TA, QTEE offers two interfaces:
> one where the TA binary is in UBUF and another where the TA binary is in a memory object.
Is this a limitation in the QTEE backend driver or on the secure side?
Can it be fixed? I don't ask for changes in the ABI to the secure
world since I assume you haven't made such changes while this patch
set has evolved.
Cheers,
Jens
>
> Best Regards,
> Amir
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 22cc7d624b0c..bc862a11d437 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
> params[n].u.value.b = ip.b;
> params[n].u.value.c = ip.c;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> + params[n].u.ubuf.size = ip.b;
> +
> + if (!access_ok(params[n].u.ubuf.uaddr,
> + params[n].u.ubuf.size))
> + return -EFAULT;
> +
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
> put_user(p->u.value.c, &up->c))
> return -EFAULT;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + if (put_user((u64)p->u.ubuf.size, &up->b))
> + return -EFAULT;
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> if (put_user((u64)p->u.memref.size, &up->b))
> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
> ip.b = p->u.value.b;
> ip.c = p->u.value.c;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + ip.a = (u64)p->u.ubuf.uaddr;
> + ip.b = p->u.ubuf.size;
> + ip.c = 0;
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
> p->u.value.b = ip.b;
> p->u.value.c = ip.c;
> break;
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> + p->u.ubuf.size = ip.b;
> +
> + if (!access_ok(params[n].u.ubuf.uaddr,
> + params[n].u.ubuf.size))
> + return -EFAULT;
> +
> + break;
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> /*
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index ce23fd42c5d4..d773f91c6bdd 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -82,6 +82,11 @@ struct tee_param_memref {
> struct tee_shm *shm;
> };
>
> +struct tee_param_ubuf {
> + void * __user uaddr;
> + size_t size;
> +};
> +
> struct tee_param_value {
> u64 a;
> u64 b;
> @@ -92,6 +97,7 @@ struct tee_param {
> u64 attr;
> union {
> struct tee_param_memref memref;
> + struct tee_param_ubuf ubuf;
> struct tee_param_value value;
> } u;
> };
> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> index d0430bee8292..3e9b1ec5dfde 100644
> --- a/include/uapi/linux/tee.h
> +++ b/include/uapi/linux/tee.h
> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
>
> +/*
> + * These defines userspace buffer parameters.
> + */
> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
> +
> /*
> * Mask for the type part of the attribute, leaves room for more types
> */
> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
> /**
> * struct tee_ioctl_param - parameter
> * @attr: attributes
> - * @a: if a memref, offset into the shared memory object, else a value parameter
> - * @b: if a memref, size of the buffer, else a value parameter
> + * @a: if a memref, offset into the shared memory object,
> + * else if a ubuf, address of the user buffer,
> + * else a value parameter
> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
> * @c: if a memref, shared memory identifier, else a value parameter
> *
> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
> - * indicates that none of the members are used.
> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
> + * are used.
> *
> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
> * identifier representing the shared memory object. A memref can reference
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
[not found] < <CAHUa44F-t29Hu0o3+0vFLjtrnA8ZGycPFcUTXEOmms9B=cZ6XA@mail.gmail.com>
@ 2025-04-09 7:20 ` Amirreza Zarrabi
2025-04-09 8:27 ` Jens Wiklander
2025-04-21 13:17 ` Sumit Garg
0 siblings, 2 replies; 9+ messages in thread
From: Amirreza Zarrabi @ 2025-04-09 7:20 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 8711 bytes --]
On 4/9/2025 4:41 PM, Jens Wiklander wrote:
> Hi Amirreza,
>
> On Wed, Apr 9, 2025 at 2:28 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> Hi jens,
>>
>> On 4/8/2025 10:19 PM, Jens Wiklander wrote:
>>
>> Hi Amirreza,
>>
>> On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
>> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> For drivers that can transfer data to the TEE without using shared
>> memory from client, it is necessary to receive the user address
>> directly, bypassing any processing by the TEE subsystem. Introduce
>> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
>> userspace buffers.
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
>> include/linux/tee_drv.h | 6 ++++++
>> include/uapi/linux/tee.h | 22 ++++++++++++++++------
>> 3 files changed, 55 insertions(+), 6 deletions(-)
>>
>> Is this patch needed now that the QCOMTEE driver supports shared
>> memory? I prefer keeping changes to the ABI to a minimum.
>>
>> Cheers,
>> Jens
>>
>> Unfortunately, this is still required. QTEE supports two types of data transfer:
>> (1) using UBUF and (2) memory objects. Even with memory object support, some APIs still
>> expect to receive data using UBUF. For instance, to load a TA, QTEE offers two interfaces:
>> one where the TA binary is in UBUF and another where the TA binary is in a memory object.
>
> Is this a limitation in the QTEE backend driver or on the secure side?
> Can it be fixed? I don't ask for changes in the ABI to the secure
> world since I assume you haven't made such changes while this patch
> set has evolved.
>
> Cheers,
> Jens
The secure-side ABI supports passing data using memcpy to the same
buffer that contains the message for QTEE, rather than using a memory
object. Some services tend to use this approach for small data instead
of allocating a memory object. I have no choice but to expose this support.
Throughout the patchset, I have not made any change to the ABI but
tried to provide support for the memory object in a separate,
independent commit, distinct from the UBUF.
Best regards,
Amir
>
>>
>> Best Regards,
>> Amir
>>
>> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
>> index 22cc7d624b0c..bc862a11d437 100644
>> --- a/drivers/tee/tee_core.c
>> +++ b/drivers/tee/tee_core.c
>> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
>> params[n].u.value.b = ip.b;
>> params[n].u.value.c = ip.c;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
>> + params[n].u.ubuf.size = ip.b;
>> +
>> + if (!access_ok(params[n].u.ubuf.uaddr,
>> + params[n].u.ubuf.size))
>> + return -EFAULT;
>> +
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
>> put_user(p->u.value.c, &up->c))
>> return -EFAULT;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + if (put_user((u64)p->u.ubuf.size, &up->b))
>> + return -EFAULT;
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> if (put_user((u64)p->u.memref.size, &up->b))
>> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
>> ip.b = p->u.value.b;
>> ip.c = p->u.value.c;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + ip.a = (u64)p->u.ubuf.uaddr;
>> + ip.b = p->u.ubuf.size;
>> + ip.c = 0;
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
>> p->u.value.b = ip.b;
>> p->u.value.c = ip.c;
>> break;
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
>> + p->u.ubuf.size = ip.b;
>> +
>> + if (!access_ok(params[n].u.ubuf.uaddr,
>> + params[n].u.ubuf.size))
>> + return -EFAULT;
>> +
>> + break;
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> /*
>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
>> index ce23fd42c5d4..d773f91c6bdd 100644
>> --- a/include/linux/tee_drv.h
>> +++ b/include/linux/tee_drv.h
>> @@ -82,6 +82,11 @@ struct tee_param_memref {
>> struct tee_shm *shm;
>> };
>>
>> +struct tee_param_ubuf {
>> + void * __user uaddr;
>> + size_t size;
>> +};
>> +
>> struct tee_param_value {
>> u64 a;
>> u64 b;
>> @@ -92,6 +97,7 @@ struct tee_param {
>> u64 attr;
>> union {
>> struct tee_param_memref memref;
>> + struct tee_param_ubuf ubuf;
>> struct tee_param_value value;
>> } u;
>> };
>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>> index d0430bee8292..3e9b1ec5dfde 100644
>> --- a/include/uapi/linux/tee.h
>> +++ b/include/uapi/linux/tee.h
>> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
>> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
>> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
>>
>> +/*
>> + * These defines userspace buffer parameters.
>> + */
>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
>> +
>> /*
>> * Mask for the type part of the attribute, leaves room for more types
>> */
>> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
>> /**
>> * struct tee_ioctl_param - parameter
>> * @attr: attributes
>> - * @a: if a memref, offset into the shared memory object, else a value parameter
>> - * @b: if a memref, size of the buffer, else a value parameter
>> + * @a: if a memref, offset into the shared memory object,
>> + * else if a ubuf, address of the user buffer,
>> + * else a value parameter
>> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
>> * @c: if a memref, shared memory identifier, else a value parameter
>> *
>> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
>> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
>> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
>> - * indicates that none of the members are used.
>> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
>> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
>> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
>> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
>> + * are used.
>> *
>> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
>> * identifier representing the shared memory object. A memref can reference
>>
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
2025-04-09 7:20 ` [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF Amirreza Zarrabi
@ 2025-04-09 8:27 ` Jens Wiklander
2025-04-21 13:17 ` Sumit Garg
1 sibling, 0 replies; 9+ messages in thread
From: Jens Wiklander @ 2025-04-09 8:27 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 9261 bytes --]
On Wed, Apr 9, 2025 at 9:20 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
>
>
> On 4/9/2025 4:41 PM, Jens Wiklander wrote:
> > Hi Amirreza,
> >
> > On Wed, Apr 9, 2025 at 2:28 AM Amirreza Zarrabi
> > <amirreza.zarrabi@oss.qualcomm.com> wrote:
> >>
> >> Hi jens,
> >>
> >> On 4/8/2025 10:19 PM, Jens Wiklander wrote:
> >>
> >> Hi Amirreza,
> >>
> >> On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
> >> <amirreza.zarrabi@oss.qualcomm.com> wrote:
> >>
> >> For drivers that can transfer data to the TEE without using shared
> >> memory from client, it is necessary to receive the user address
> >> directly, bypassing any processing by the TEE subsystem. Introduce
> >> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
> >> userspace buffers.
> >>
> >> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> >> ---
> >> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
> >> include/linux/tee_drv.h | 6 ++++++
> >> include/uapi/linux/tee.h | 22 ++++++++++++++++------
> >> 3 files changed, 55 insertions(+), 6 deletions(-)
> >>
> >> Is this patch needed now that the QCOMTEE driver supports shared
> >> memory? I prefer keeping changes to the ABI to a minimum.
> >>
> >> Cheers,
> >> Jens
> >>
> >> Unfortunately, this is still required. QTEE supports two types of data transfer:
> >> (1) using UBUF and (2) memory objects. Even with memory object support, some APIs still
> >> expect to receive data using UBUF. For instance, to load a TA, QTEE offers two interfaces:
> >> one where the TA binary is in UBUF and another where the TA binary is in a memory object.
> >
> > Is this a limitation in the QTEE backend driver or on the secure side?
> > Can it be fixed? I don't ask for changes in the ABI to the secure
> > world since I assume you haven't made such changes while this patch
> > set has evolved.
> >
> > Cheers,
> > Jens
>
> The secure-side ABI supports passing data using memcpy to the same
> buffer that contains the message for QTEE, rather than using a memory
> object. Some services tend to use this approach for small data instead
> of allocating a memory object. I have no choice but to expose this support.
Got it, thanks! It's needed.
>
> Throughout the patchset, I have not made any change to the ABI but
> tried to provide support for the memory object in a separate,
> independent commit, distinct from the UBUF.
OK
Cheers,
Jens
>
> Best regards,
> Amir
>
> >
> >>
> >> Best Regards,
> >> Amir
> >>
> >> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> >> index 22cc7d624b0c..bc862a11d437 100644
> >> --- a/drivers/tee/tee_core.c
> >> +++ b/drivers/tee/tee_core.c
> >> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
> >> params[n].u.value.b = ip.b;
> >> params[n].u.value.c = ip.c;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> >> + params[n].u.ubuf.size = ip.b;
> >> +
> >> + if (!access_ok(params[n].u.ubuf.uaddr,
> >> + params[n].u.ubuf.size))
> >> + return -EFAULT;
> >> +
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
> >> put_user(p->u.value.c, &up->c))
> >> return -EFAULT;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + if (put_user((u64)p->u.ubuf.size, &up->b))
> >> + return -EFAULT;
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> if (put_user((u64)p->u.memref.size, &up->b))
> >> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
> >> ip.b = p->u.value.b;
> >> ip.c = p->u.value.c;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + ip.a = (u64)p->u.ubuf.uaddr;
> >> + ip.b = p->u.ubuf.size;
> >> + ip.c = 0;
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
> >> p->u.value.b = ip.b;
> >> p->u.value.c = ip.c;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> >> + p->u.ubuf.size = ip.b;
> >> +
> >> + if (!access_ok(params[n].u.ubuf.uaddr,
> >> + params[n].u.ubuf.size))
> >> + return -EFAULT;
> >> +
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> /*
> >> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> >> index ce23fd42c5d4..d773f91c6bdd 100644
> >> --- a/include/linux/tee_drv.h
> >> +++ b/include/linux/tee_drv.h
> >> @@ -82,6 +82,11 @@ struct tee_param_memref {
> >> struct tee_shm *shm;
> >> };
> >>
> >> +struct tee_param_ubuf {
> >> + void * __user uaddr;
> >> + size_t size;
> >> +};
> >> +
> >> struct tee_param_value {
> >> u64 a;
> >> u64 b;
> >> @@ -92,6 +97,7 @@ struct tee_param {
> >> u64 attr;
> >> union {
> >> struct tee_param_memref memref;
> >> + struct tee_param_ubuf ubuf;
> >> struct tee_param_value value;
> >> } u;
> >> };
> >> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> >> index d0430bee8292..3e9b1ec5dfde 100644
> >> --- a/include/uapi/linux/tee.h
> >> +++ b/include/uapi/linux/tee.h
> >> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
> >> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
> >> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
> >>
> >> +/*
> >> + * These defines userspace buffer parameters.
> >> + */
> >> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
> >> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
> >> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
> >> +
> >> /*
> >> * Mask for the type part of the attribute, leaves room for more types
> >> */
> >> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
> >> /**
> >> * struct tee_ioctl_param - parameter
> >> * @attr: attributes
> >> - * @a: if a memref, offset into the shared memory object, else a value parameter
> >> - * @b: if a memref, size of the buffer, else a value parameter
> >> + * @a: if a memref, offset into the shared memory object,
> >> + * else if a ubuf, address of the user buffer,
> >> + * else a value parameter
> >> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
> >> * @c: if a memref, shared memory identifier, else a value parameter
> >> *
> >> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
> >> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
> >> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
> >> - * indicates that none of the members are used.
> >> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
> >> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
> >> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
> >> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
> >> + * are used.
> >> *
> >> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
> >> * identifier representing the shared memory object. A memref can reference
> >>
> >> --
> >> 2.34.1
> >>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
2025-04-09 7:20 ` [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF Amirreza Zarrabi
2025-04-09 8:27 ` Jens Wiklander
@ 2025-04-21 13:17 ` Sumit Garg
2025-04-28 4:37 ` Amirreza Zarrabi
1 sibling, 1 reply; 9+ messages in thread
From: Sumit Garg @ 2025-04-21 13:17 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 9893 bytes --]
On Wed, Apr 09, 2025 at 05:20:08PM +1000, Amirreza Zarrabi wrote:
>
>
> On 4/9/2025 4:41 PM, Jens Wiklander wrote:
> > Hi Amirreza,
> >
> > On Wed, Apr 9, 2025 at 2:28 AM Amirreza Zarrabi
> > <amirreza.zarrabi@oss.qualcomm.com> wrote:
> >>
> >> Hi jens,
> >>
> >> On 4/8/2025 10:19 PM, Jens Wiklander wrote:
> >>
> >> Hi Amirreza,
> >>
> >> On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
> >> <amirreza.zarrabi@oss.qualcomm.com> wrote:
> >>
> >> For drivers that can transfer data to the TEE without using shared
> >> memory from client, it is necessary to receive the user address
> >> directly, bypassing any processing by the TEE subsystem. Introduce
> >> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
> >> userspace buffers.
> >>
> >> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> >> ---
> >> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
> >> include/linux/tee_drv.h | 6 ++++++
> >> include/uapi/linux/tee.h | 22 ++++++++++++++++------
> >> 3 files changed, 55 insertions(+), 6 deletions(-)
> >>
> >> Is this patch needed now that the QCOMTEE driver supports shared
> >> memory? I prefer keeping changes to the ABI to a minimum.
> >>
> >> Cheers,
> >> Jens
> >>
> >> Unfortunately, this is still required. QTEE supports two types of data transfer:
> >> (1) using UBUF and (2) memory objects. Even with memory object support, some APIs still
> >> expect to receive data using UBUF. For instance, to load a TA, QTEE offers two interfaces:
> >> one where the TA binary is in UBUF and another where the TA binary is in a memory object.
> >
> > Is this a limitation in the QTEE backend driver or on the secure side?
> > Can it be fixed? I don't ask for changes in the ABI to the secure
> > world since I assume you haven't made such changes while this patch
> > set has evolved.
> >
> > Cheers,
> > Jens
>
> The secure-side ABI supports passing data using memcpy to the same
> buffer that contains the message for QTEE, rather than using a memory
> object. Some services tend to use this approach for small data instead
> of allocating a memory object. I have no choice but to expose this support.
Okay, I can see how QTEE supports embedding user buffers in fixed size
shared memory buffers allocated by the driver with maximum size limits.
OP-TEE also have support for temporary shared memory where the user
space client directly passes the buffer to share with OP-TEE. Then the
libteec [1] handles the underneath copy to and from the shared memory
allocation automatically.
So is there a limitation for QCOMTEE user space library [2] to do the
same? This way we will be able to retain the user-space ABI as well as
simplicify the kernel driver.
[1] https://github.com/OP-TEE/optee_client/blob/master/libteec/src/tee_client_api.c#L365
[2] https://github.com/quic/quic-teec
-Sumit
>
> Throughout the patchset, I have not made any change to the ABI but
> tried to provide support for the memory object in a separate,
> independent commit, distinct from the UBUF.
>
> Best regards,
> Amir
>
> >
> >>
> >> Best Regards,
> >> Amir
> >>
> >> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> >> index 22cc7d624b0c..bc862a11d437 100644
> >> --- a/drivers/tee/tee_core.c
> >> +++ b/drivers/tee/tee_core.c
> >> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
> >> params[n].u.value.b = ip.b;
> >> params[n].u.value.c = ip.c;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> >> + params[n].u.ubuf.size = ip.b;
> >> +
> >> + if (!access_ok(params[n].u.ubuf.uaddr,
> >> + params[n].u.ubuf.size))
> >> + return -EFAULT;
> >> +
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
> >> put_user(p->u.value.c, &up->c))
> >> return -EFAULT;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + if (put_user((u64)p->u.ubuf.size, &up->b))
> >> + return -EFAULT;
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> if (put_user((u64)p->u.memref.size, &up->b))
> >> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
> >> ip.b = p->u.value.b;
> >> ip.c = p->u.value.c;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + ip.a = (u64)p->u.ubuf.uaddr;
> >> + ip.b = p->u.ubuf.size;
> >> + ip.c = 0;
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
> >> p->u.value.b = ip.b;
> >> p->u.value.c = ip.c;
> >> break;
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
> >> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
> >> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
> >> + p->u.ubuf.size = ip.b;
> >> +
> >> + if (!access_ok(params[n].u.ubuf.uaddr,
> >> + params[n].u.ubuf.size))
> >> + return -EFAULT;
> >> +
> >> + break;
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> >> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> >> /*
> >> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> >> index ce23fd42c5d4..d773f91c6bdd 100644
> >> --- a/include/linux/tee_drv.h
> >> +++ b/include/linux/tee_drv.h
> >> @@ -82,6 +82,11 @@ struct tee_param_memref {
> >> struct tee_shm *shm;
> >> };
> >>
> >> +struct tee_param_ubuf {
> >> + void * __user uaddr;
> >> + size_t size;
> >> +};
> >> +
> >> struct tee_param_value {
> >> u64 a;
> >> u64 b;
> >> @@ -92,6 +97,7 @@ struct tee_param {
> >> u64 attr;
> >> union {
> >> struct tee_param_memref memref;
> >> + struct tee_param_ubuf ubuf;
> >> struct tee_param_value value;
> >> } u;
> >> };
> >> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> >> index d0430bee8292..3e9b1ec5dfde 100644
> >> --- a/include/uapi/linux/tee.h
> >> +++ b/include/uapi/linux/tee.h
> >> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
> >> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
> >> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
> >>
> >> +/*
> >> + * These defines userspace buffer parameters.
> >> + */
> >> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
> >> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
> >> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
> >> +
> >> /*
> >> * Mask for the type part of the attribute, leaves room for more types
> >> */
> >> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
> >> /**
> >> * struct tee_ioctl_param - parameter
> >> * @attr: attributes
> >> - * @a: if a memref, offset into the shared memory object, else a value parameter
> >> - * @b: if a memref, size of the buffer, else a value parameter
> >> + * @a: if a memref, offset into the shared memory object,
> >> + * else if a ubuf, address of the user buffer,
> >> + * else a value parameter
> >> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
> >> * @c: if a memref, shared memory identifier, else a value parameter
> >> *
> >> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
> >> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
> >> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
> >> - * indicates that none of the members are used.
> >> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
> >> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
> >> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
> >> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
> >> + * are used.
> >> *
> >> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
> >> * identifier representing the shared memory object. A memref can reference
> >>
> >> --
> >> 2.34.1
> >>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
2025-04-21 13:17 ` Sumit Garg
@ 2025-04-28 4:37 ` Amirreza Zarrabi
0 siblings, 0 replies; 9+ messages in thread
From: Amirreza Zarrabi @ 2025-04-28 4:37 UTC (permalink / raw)
To: op-tee
[-- Attachment #1: Type: text/plain, Size: 10406 bytes --]
Hi Sumit,
On 4/21/2025 11:17 PM, Sumit Garg wrote:
> On Wed, Apr 09, 2025 at 05:20:08PM +1000, Amirreza Zarrabi wrote:
>>
>>
>> On 4/9/2025 4:41 PM, Jens Wiklander wrote:
>>> Hi Amirreza,
>>>
>>> On Wed, Apr 9, 2025 at 2:28 AM Amirreza Zarrabi
>>> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>>>
>>>> Hi jens,
>>>>
>>>> On 4/8/2025 10:19 PM, Jens Wiklander wrote:
>>>>
>>>> Hi Amirreza,
>>>>
>>>> On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
>>>> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>>>
>>>> For drivers that can transfer data to the TEE without using shared
>>>> memory from client, it is necessary to receive the user address
>>>> directly, bypassing any processing by the TEE subsystem. Introduce
>>>> TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent
>>>> userspace buffers.
>>>>
>>>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>>>> ---
>>>> drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++
>>>> include/linux/tee_drv.h | 6 ++++++
>>>> include/uapi/linux/tee.h | 22 ++++++++++++++++------
>>>> 3 files changed, 55 insertions(+), 6 deletions(-)
>>>>
>>>> Is this patch needed now that the QCOMTEE driver supports shared
>>>> memory? I prefer keeping changes to the ABI to a minimum.
>>>>
>>>> Cheers,
>>>> Jens
>>>>
>>>> Unfortunately, this is still required. QTEE supports two types of data transfer:
>>>> (1) using UBUF and (2) memory objects. Even with memory object support, some APIs still
>>>> expect to receive data using UBUF. For instance, to load a TA, QTEE offers two interfaces:
>>>> one where the TA binary is in UBUF and another where the TA binary is in a memory object.
>>>
>>> Is this a limitation in the QTEE backend driver or on the secure side?
>>> Can it be fixed? I don't ask for changes in the ABI to the secure
>>> world since I assume you haven't made such changes while this patch
>>> set has evolved.
>>>
>>> Cheers,
>>> Jens
>>
>> The secure-side ABI supports passing data using memcpy to the same
>> buffer that contains the message for QTEE, rather than using a memory
>> object. Some services tend to use this approach for small data instead
>> of allocating a memory object. I have no choice but to expose this support.
>
> Okay, I can see how QTEE supports embedding user buffers in fixed size
> shared memory buffers allocated by the driver with maximum size limits.
>
> OP-TEE also have support for temporary shared memory where the user
> space client directly passes the buffer to share with OP-TEE. Then the
> libteec [1] handles the underneath copy to and from the shared memory
> allocation automatically.
>
> So is there a limitation for QCOMTEE user space library [2] to do the
> same? This way we will be able to retain the user-space ABI as well as
> simplicify the kernel driver.
>
> [1] https://github.com/OP-TEE/optee_client/blob/master/libteec/src/tee_client_api.c#L365
> [2] https://github.com/quic/quic-teec
>
> -Sumit
>
Unfortunately, I do not have control over the TA's API. If a TA expects
to receive data in the embedded buffer, then I cannot use a memory
object. To maintain the ABI and avoid the UBUF, I need two copies:
(1) the user data passed to the library is copied to memory allocated
using TEE_ALLOC, (2) in the driver, the data is copied from the tee_shm to
the embedded buffer as expected by QTEE.
- Amir
>>
>> Throughout the patchset, I have not made any change to the ABI but
>> tried to provide support for the memory object in a separate,
>> independent commit, distinct from the UBUF.
>>
>> Best regards,
>> Amir
>>
>>>
>>>>
>>>> Best Regards,
>>>> Amir
>>>>
>>>> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
>>>> index 22cc7d624b0c..bc862a11d437 100644
>>>> --- a/drivers/tee/tee_core.c
>>>> +++ b/drivers/tee/tee_core.c
>>>> @@ -404,6 +404,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params,
>>>> params[n].u.value.b = ip.b;
>>>> params[n].u.value.c = ip.c;
>>>> break;
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>>>> + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a);
>>>> + params[n].u.ubuf.size = ip.b;
>>>> +
>>>> + if (!access_ok(params[n].u.ubuf.uaddr,
>>>> + params[n].u.ubuf.size))
>>>> + return -EFAULT;
>>>> +
>>>> + break;
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>>>> @@ -472,6 +483,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams,
>>>> put_user(p->u.value.c, &up->c))
>>>> return -EFAULT;
>>>> break;
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>>>> + if (put_user((u64)p->u.ubuf.size, &up->b))
>>>> + return -EFAULT;
>>>> + break;
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>>>> if (put_user((u64)p->u.memref.size, &up->b))
>>>> @@ -672,6 +688,13 @@ static int params_to_supp(struct tee_context *ctx,
>>>> ip.b = p->u.value.b;
>>>> ip.c = p->u.value.c;
>>>> break;
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>>>> + ip.a = (u64)p->u.ubuf.uaddr;
>>>> + ip.b = p->u.ubuf.size;
>>>> + ip.c = 0;
>>>> + break;
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>>>> @@ -774,6 +797,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params,
>>>> p->u.value.b = ip.b;
>>>> p->u.value.c = ip.c;
>>>> break;
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
>>>> + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT:
>>>> + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a);
>>>> + p->u.ubuf.size = ip.b;
>>>> +
>>>> + if (!access_ok(params[n].u.ubuf.uaddr,
>>>> + params[n].u.ubuf.size))
>>>> + return -EFAULT;
>>>> +
>>>> + break;
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>>>> case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>>>> /*
>>>> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
>>>> index ce23fd42c5d4..d773f91c6bdd 100644
>>>> --- a/include/linux/tee_drv.h
>>>> +++ b/include/linux/tee_drv.h
>>>> @@ -82,6 +82,11 @@ struct tee_param_memref {
>>>> struct tee_shm *shm;
>>>> };
>>>>
>>>> +struct tee_param_ubuf {
>>>> + void * __user uaddr;
>>>> + size_t size;
>>>> +};
>>>> +
>>>> struct tee_param_value {
>>>> u64 a;
>>>> u64 b;
>>>> @@ -92,6 +97,7 @@ struct tee_param {
>>>> u64 attr;
>>>> union {
>>>> struct tee_param_memref memref;
>>>> + struct tee_param_ubuf ubuf;
>>>> struct tee_param_value value;
>>>> } u;
>>>> };
>>>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>>>> index d0430bee8292..3e9b1ec5dfde 100644
>>>> --- a/include/uapi/linux/tee.h
>>>> +++ b/include/uapi/linux/tee.h
>>>> @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data {
>>>> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6
>>>> #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */
>>>>
>>>> +/*
>>>> + * These defines userspace buffer parameters.
>>>> + */
>>>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8
>>>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9
>>>> +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */
>>>> +
>>>> /*
>>>> * Mask for the type part of the attribute, leaves room for more types
>>>> */
>>>> @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data {
>>>> /**
>>>> * struct tee_ioctl_param - parameter
>>>> * @attr: attributes
>>>> - * @a: if a memref, offset into the shared memory object, else a value parameter
>>>> - * @b: if a memref, size of the buffer, else a value parameter
>>>> + * @a: if a memref, offset into the shared memory object,
>>>> + * else if a ubuf, address of the user buffer,
>>>> + * else a value parameter
>>>> + * @b: if a memref or ubuf, size of the buffer, else a value parameter
>>>> * @c: if a memref, shared memory identifier, else a value parameter
>>>> *
>>>> - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
>>>> - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
>>>> - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
>>>> - * indicates that none of the members are used.
>>>> + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is
>>>> + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value,
>>>> + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_*
>>>> + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members
>>>> + * are used.
>>>> *
>>>> * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an
>>>> * identifier representing the shared memory object. A memref can reference
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-28 4:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] < <CAHUa44F-t29Hu0o3+0vFLjtrnA8ZGycPFcUTXEOmms9B=cZ6XA@mail.gmail.com>
2025-04-09 7:20 ` [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF Amirreza Zarrabi
2025-04-09 8:27 ` Jens Wiklander
2025-04-21 13:17 ` Sumit Garg
2025-04-28 4:37 ` Amirreza Zarrabi
[not found] < <CAHUa44GRBiRr6CsFWxJhyzf1cRSEP66m5K7uFntOv3oYWTHWgQ@mail.gmail.com>
2025-04-09 0:28 ` Amirreza Zarrabi
2025-04-09 6:41 ` Jens Wiklander
[not found] <=?utf-8?q?=3C20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-?= =?utf-8?q?3-7f457073282d=40oss=2Equalcomm=2Ecom=3E?=>
2025-03-29 4:58 ` kernel test robot
2025-04-08 12:19 ` Jens Wiklander
[not found] <=?utf-8?q?=3C20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-?= =?utf-8?q?0-7f457073282d=40oss=2Equalcomm=2Ecom=3E?=>
2025-03-28 2:47 ` Amirreza Zarrabi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox