Earlier this patch-set was part of TEE Trusted keys patch-set [1]. But since these are completely independent enhancements for TEE kernel client interface which can be merged separately while TEE Trusted keys discussions are ongoing.
Patch #1 enables support for registered kernel shared memory with TEE.
Patch #2 enables support for private kernel login method required for cases like trusted keys where we don't wan't user-space to directly access TEE service.
[1] https://lkml.org/lkml/2019/10/31/430
Changes in v5: - Misc. renaming of variables.
Sumit Garg (2): tee: enable support to register kernel memory tee: add private login method for kernel clients
drivers/tee/tee_core.c | 6 ++++++ drivers/tee/tee_shm.c | 28 +++++++++++++++++++++++++--- include/linux/tee_drv.h | 1 + include/uapi/linux/tee.h | 8 ++++++++ 4 files changed, 40 insertions(+), 3 deletions(-)
Enable support to register kernel memory reference with TEE. This change will allow TEE bus drivers to register memory references.
Signed-off-by: Sumit Garg sumit.garg@linaro.org --- drivers/tee/tee_shm.c | 28 +++++++++++++++++++++++++--- include/linux/tee_drv.h | 1 + 2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 937ac5a..a6c75a4 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -9,6 +9,7 @@ #include <linux/sched.h> #include <linux/slab.h> #include <linux/tee_drv.h> +#include <linux/uio.h> #include "tee_private.h"
static void tee_shm_release(struct tee_shm *shm) @@ -217,14 +218,15 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, size_t length, u32 flags) { struct tee_device *teedev = ctx->teedev; - const u32 req_flags = TEE_SHM_DMA_BUF | TEE_SHM_USER_MAPPED; + const u32 req_user_flags = TEE_SHM_DMA_BUF | TEE_SHM_USER_MAPPED; + const u32 req_kernel_flags = TEE_SHM_DMA_BUF | TEE_SHM_KERNEL_MAPPED; struct tee_shm *shm; void *ret; int rc; int num_pages; unsigned long start;
- if (flags != req_flags) + if (flags != req_user_flags && flags != req_kernel_flags) return ERR_PTR(-ENOTSUPP);
if (!tee_device_get(teedev)) @@ -259,7 +261,27 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, goto err; }
- rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages); + if (flags & TEE_SHM_USER_MAPPED) { + rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, + shm->pages); + } else { + struct kvec *kiov; + int i; + + kiov = kcalloc(num_pages, sizeof(*kiov), GFP_KERNEL); + if (!kiov) { + ret = ERR_PTR(-ENOMEM); + goto err; + } + + for (i = 0; i < num_pages; i++) { + kiov[i].iov_base = (void *)(start + i * PAGE_SIZE); + kiov[i].iov_len = PAGE_SIZE; + } + + rc = get_kernel_pages(kiov, num_pages, 0, shm->pages); + kfree(kiov); + } if (rc > 0) shm->num_pages = rc; if (rc != num_pages) { diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 7a03f68..dedf8fa 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -26,6 +26,7 @@ #define TEE_SHM_REGISTER BIT(3) /* Memory registered in secure world */ #define TEE_SHM_USER_MAPPED BIT(4) /* Memory mapped in user space */ #define TEE_SHM_POOL BIT(5) /* Memory allocated from pool */ +#define TEE_SHM_KERNEL_MAPPED BIT(6) /* Memory mapped in kernel space */
struct device; struct tee_device;
There are use-cases where user-space shouldn't be allowed to communicate directly with a TEE device which is dedicated to provide a specific service for a kernel client. So add a private login method for kernel clients and disallow user-space to open-session using GP implementation defined login method range: (0x80000000 - 0xFFFFFFFF).
Signed-off-by: Sumit Garg sumit.garg@linaro.org --- drivers/tee/tee_core.c | 6 ++++++ include/uapi/linux/tee.h | 8 ++++++++ 2 files changed, 14 insertions(+)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 37d22e3..533e7a8 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -334,6 +334,12 @@ static int tee_ioctl_open_session(struct tee_context *ctx, goto out; }
+ if (arg.clnt_login & TEE_IOCTL_LOGIN_MASK) { + pr_debug("login method not allowed for user-space client\n"); + rc = -EPERM; + goto out; + } + rc = ctx->teedev->desc->ops->open_session(ctx, &arg, params); if (rc) goto out; diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 6596f3a..19172a2 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -173,6 +173,14 @@ struct tee_ioctl_buf_data { #define TEE_IOCTL_LOGIN_APPLICATION 4 #define TEE_IOCTL_LOGIN_USER_APPLICATION 5 #define TEE_IOCTL_LOGIN_GROUP_APPLICATION 6 +/* + * Disallow user-space to use GP implementation specific login + * method range (0x80000000 - 0xFFFFFFFF). This range is rather + * being reserved for REE kernel clients or TEE implementation. + */ +#define TEE_IOCTL_LOGIN_MASK 0x80000000 +/* Private login method for REE kernel clients */ +#define TEE_IOCTL_LOGIN_REE_KERNEL 0x80000000
/** * struct tee_ioctl_param - parameter
On Thu, Mar 26, 2020 at 8:24 AM Sumit Garg sumit.garg@linaro.org wrote:
There are use-cases where user-space shouldn't be allowed to communicate directly with a TEE device which is dedicated to provide a specific service for a kernel client. So add a private login method for kernel clients
OK
and disallow user-space to open-session using GP implementation
defined login method range: (0x80000000 - 0xFFFFFFFF).
I'm not sure this is correct, because it would prevent the client library or the TEE supplicant from using such values, although they are part of the TEE implementation; and further, nothing mandates that an implementation-defined method should not be used directly by client applications.
On Thu, 26 Mar 2020 at 14:05, Jérôme Forissier jerome@forissier.org wrote:
On Thu, Mar 26, 2020 at 8:24 AM Sumit Garg sumit.garg@linaro.org wrote:
There are use-cases where user-space shouldn't be allowed to communicate directly with a TEE device which is dedicated to provide a specific service for a kernel client. So add a private login method for kernel clients
OK
and disallow user-space to open-session using GP implementation defined login method range: (0x80000000 - 0xFFFFFFFF).
I'm not sure this is correct, because it would prevent the client library or the TEE supplicant from using such values, although they are part of the TEE implementation; and further, nothing mandates that an implementation-defined method should not be used directly by client applications.
Initial implementation of this patch only put restriction for single implementation-defined login method (TEE_IOCTL_LOGIN_REE_KERNEL) only. But after discussion with Jens here [1], I have changed that to restrict complete implementation-defined range. If we think to further partition this range considering API stability then I am open to that too.
[1] https://lore.kernel.org/patchwork/patch/1088062/
-Sumit
-- Jerome
Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/tee_core.c | 6 ++++++ include/uapi/linux/tee.h | 8 ++++++++ 2 files changed, 14 insertions(+)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 37d22e3..533e7a8 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -334,6 +334,12 @@ static int tee_ioctl_open_session(struct tee_context *ctx, goto out; }
if (arg.clnt_login & TEE_IOCTL_LOGIN_MASK) {pr_debug("login method not allowed for user-space client\n");rc = -EPERM;goto out;}rc = ctx->teedev->desc->ops->open_session(ctx, &arg, params); if (rc) goto out;diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 6596f3a..19172a2 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -173,6 +173,14 @@ struct tee_ioctl_buf_data { #define TEE_IOCTL_LOGIN_APPLICATION 4 #define TEE_IOCTL_LOGIN_USER_APPLICATION 5 #define TEE_IOCTL_LOGIN_GROUP_APPLICATION 6 +/*
- Disallow user-space to use GP implementation specific login
- method range (0x80000000 - 0xFFFFFFFF). This range is rather
- being reserved for REE kernel clients or TEE implementation.
- */
+#define TEE_IOCTL_LOGIN_MASK 0x80000000 +/* Private login method for REE kernel clients */ +#define TEE_IOCTL_LOGIN_REE_KERNEL 0x80000000
/**
- struct tee_ioctl_param - parameter
-- 2.7.4
Tee-dev mailing list Tee-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/tee-dev
On 3/26/20 10:07 AM, Sumit Garg wrote:
On Thu, 26 Mar 2020 at 14:05, Jérôme Forissier jerome@forissier.org wrote:
On Thu, Mar 26, 2020 at 8:24 AM Sumit Garg sumit.garg@linaro.org wrote:
There are use-cases where user-space shouldn't be allowed to communicate directly with a TEE device which is dedicated to provide a specific service for a kernel client. So add a private login method for kernel clients
OK
and disallow user-space to open-session using GP implementation defined login method range: (0x80000000 - 0xFFFFFFFF).
I'm not sure this is correct, because it would prevent the client library or the TEE supplicant from using such values, although they are part of the TEE implementation; and further, nothing mandates that an implementation-defined method should not be used directly by client applications.
Initial implementation of this patch only put restriction for single implementation-defined login method (TEE_IOCTL_LOGIN_REE_KERNEL) only. But after discussion with Jens here [1], I have changed that to restrict complete implementation-defined range. If we think to further partition this range considering API stability then I am open to that too.
In the end he proposed to reserve half the range for user space and half for kernel space.
(BTW sorry for my previous HTML reply)
On Thu, 26 Mar 2020 at 14:53, Jerome Forissier jerome@forissier.org wrote:
On 3/26/20 10:07 AM, Sumit Garg wrote:
On Thu, 26 Mar 2020 at 14:05, Jérôme Forissier jerome@forissier.org wrote:
On Thu, Mar 26, 2020 at 8:24 AM Sumit Garg sumit.garg@linaro.org wrote:
There are use-cases where user-space shouldn't be allowed to communicate directly with a TEE device which is dedicated to provide a specific service for a kernel client. So add a private login method for kernel clients
OK
and disallow user-space to open-session using GP implementation defined login method range: (0x80000000 - 0xFFFFFFFF).
I'm not sure this is correct, because it would prevent the client library or the TEE supplicant from using such values, although they are part of the TEE implementation; and further, nothing mandates that an implementation-defined method should not be used directly by client applications.
Initial implementation of this patch only put restriction for single implementation-defined login method (TEE_IOCTL_LOGIN_REE_KERNEL) only. But after discussion with Jens here [1], I have changed that to restrict complete implementation-defined range. If we think to further partition this range considering API stability then I am open to that too.
In the end he proposed to reserve half the range for user space and half for kernel space.
It seems I probably misunderstood his proposal. So let me reserve (0x80000000 - 0xBFFFFFFF) range for kernel space.
(BTW sorry for my previous HTML reply)
No worries.
-Sumit
-- Jerome
-Sumit
-- Jerome
Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/tee_core.c | 6 ++++++ include/uapi/linux/tee.h | 8 ++++++++ 2 files changed, 14 insertions(+)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 37d22e3..533e7a8 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -334,6 +334,12 @@ static int tee_ioctl_open_session(struct tee_context *ctx, goto out; }
if (arg.clnt_login & TEE_IOCTL_LOGIN_MASK) {pr_debug("login method not allowed for user-space client\n");rc = -EPERM;goto out;}rc = ctx->teedev->desc->ops->open_session(ctx, &arg, params); if (rc) goto out;diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 6596f3a..19172a2 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -173,6 +173,14 @@ struct tee_ioctl_buf_data { #define TEE_IOCTL_LOGIN_APPLICATION 4 #define TEE_IOCTL_LOGIN_USER_APPLICATION 5 #define TEE_IOCTL_LOGIN_GROUP_APPLICATION 6 +/*
- Disallow user-space to use GP implementation specific login
- method range (0x80000000 - 0xFFFFFFFF). This range is rather
- being reserved for REE kernel clients or TEE implementation.
- */
+#define TEE_IOCTL_LOGIN_MASK 0x80000000 +/* Private login method for REE kernel clients */ +#define TEE_IOCTL_LOGIN_REE_KERNEL 0x80000000
/**
- struct tee_ioctl_param - parameter
-- 2.7.4
Tee-dev mailing list Tee-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/tee-dev
-
Jerome Forissier -
Jérôme Forissier
-
Sumit Garg