* [PATCH v5 0/2] Enhance TEE kernel client interface @ 2020-03-26 7:23 Sumit Garg 2020-03-26 7:23 ` [PATCH v5 1/2] tee: enable support to register kernel memory Sumit Garg 2020-03-26 7:23 ` [PATCH v5 2/2] tee: add private login method for kernel clients Sumit Garg 0 siblings, 2 replies; 6+ messages in thread From: Sumit Garg @ 2020-03-26 7:23 UTC (permalink / raw) To: jens.wiklander Cc: tee-dev, linux-kernel, stuart.yoder, daniel.thompson, Sumit Garg 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(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/2] tee: enable support to register kernel memory 2020-03-26 7:23 [PATCH v5 0/2] Enhance TEE kernel client interface Sumit Garg @ 2020-03-26 7:23 ` Sumit Garg 2020-03-26 7:23 ` [PATCH v5 2/2] tee: add private login method for kernel clients Sumit Garg 1 sibling, 0 replies; 6+ messages in thread From: Sumit Garg @ 2020-03-26 7:23 UTC (permalink / raw) To: jens.wiklander Cc: tee-dev, linux-kernel, stuart.yoder, daniel.thompson, Sumit Garg 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; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 2/2] tee: add private login method for kernel clients 2020-03-26 7:23 [PATCH v5 0/2] Enhance TEE kernel client interface Sumit Garg 2020-03-26 7:23 ` [PATCH v5 1/2] tee: enable support to register kernel memory Sumit Garg @ 2020-03-26 7:23 ` Sumit Garg [not found] ` <CAA-cTWZWPFtq-9MOrr6YDV4SGyo_JNaNsFJc=pjaWBrWHMid1A@mail.gmail.com> 1 sibling, 1 reply; 6+ messages in thread From: Sumit Garg @ 2020-03-26 7:23 UTC (permalink / raw) To: jens.wiklander Cc: tee-dev, linux-kernel, stuart.yoder, daniel.thompson, Sumit Garg 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 -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <CAA-cTWZWPFtq-9MOrr6YDV4SGyo_JNaNsFJc=pjaWBrWHMid1A@mail.gmail.com>]
* Re: [Tee-dev] [PATCH v5 2/2] tee: add private login method for kernel clients [not found] ` <CAA-cTWZWPFtq-9MOrr6YDV4SGyo_JNaNsFJc=pjaWBrWHMid1A@mail.gmail.com> @ 2020-03-26 9:07 ` Sumit Garg 2020-03-26 9:23 ` Jerome Forissier 0 siblings, 1 reply; 6+ messages in thread From: Sumit Garg @ 2020-03-26 9:07 UTC (permalink / raw) To: Jérôme Forissier Cc: Jens Wiklander, tee-dev @ lists . linaro . org, Daniel Thompson, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tee-dev] [PATCH v5 2/2] tee: add private login method for kernel clients 2020-03-26 9:07 ` [Tee-dev] " Sumit Garg @ 2020-03-26 9:23 ` Jerome Forissier 2020-03-26 9:39 ` Sumit Garg 0 siblings, 1 reply; 6+ messages in thread From: Jerome Forissier @ 2020-03-26 9:23 UTC (permalink / raw) To: Sumit Garg Cc: Jens Wiklander, tee-dev @ lists . linaro . org, Daniel Thompson, Linux Kernel Mailing List 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. > > [1] https://lore.kernel.org/patchwork/patch/1088062/ 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) -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tee-dev] [PATCH v5 2/2] tee: add private login method for kernel clients 2020-03-26 9:23 ` Jerome Forissier @ 2020-03-26 9:39 ` Sumit Garg 0 siblings, 0 replies; 6+ messages in thread From: Sumit Garg @ 2020-03-26 9:39 UTC (permalink / raw) To: Jerome Forissier Cc: Jens Wiklander, tee-dev @ lists . linaro . org, Daniel Thompson, Linux Kernel Mailing List 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. > > > > [1] https://lore.kernel.org/patchwork/patch/1088062/ > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-26 9:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 7:23 [PATCH v5 0/2] Enhance TEE kernel client interface Sumit Garg
2020-03-26 7:23 ` [PATCH v5 1/2] tee: enable support to register kernel memory Sumit Garg
2020-03-26 7:23 ` [PATCH v5 2/2] tee: add private login method for kernel clients Sumit Garg
[not found] ` <CAA-cTWZWPFtq-9MOrr6YDV4SGyo_JNaNsFJc=pjaWBrWHMid1A@mail.gmail.com>
2020-03-26 9:07 ` [Tee-dev] " Sumit Garg
2020-03-26 9:23 ` Jerome Forissier
2020-03-26 9:39 ` Sumit Garg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).