| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHUa44GRBiRr6CsFWxJhyzf1cRSEP66m5K7uFntOv3oYWTHWgQ@mail.gmail.com>
Date: Tue, 8 Apr 2025 14:19:39 +0200
From: Jens Wiklander <jens.wiklander@...aro.org>
To: Amirreza Zarrabi <amirreza.zarrabi@....qualcomm.com>
Cc: Sumit Garg <sumit.garg@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, Apurupa Pattapu <quic_apurupa@...cinc.com>,
Kees Cook <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
Sumit Semwal <sumit.semwal@...aro.org>, Christian König <christian.koenig@....com>,
linux-arm-msm@...r.kernel.org, op-tee@...ts.trustedfirmware.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 03/11] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF
Hi Amirreza,
On Fri, Mar 28, 2025 at 3:48 AM Amirreza Zarrabi
<amirreza.zarrabi@....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@....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
>
Powered by blists - more mailing lists