diff mbox series

[v3,03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF

Message ID 20250327-qcom-tee-using-tee-ss-without-mem-obj-v3-3-7f457073282d@oss.qualcomm.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Trusted Execution Environment (TEE) driver for Qualcomm TEE (QTEE) | expand

Commit Message

Amirreza Zarrabi March 28, 2025, 2:47 a.m. UTC
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(-)

Comments

kernel test robot March 29, 2025, 4:58 a.m. UTC | #1
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@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@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@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
diff mbox series

Patch

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