We should not need to index into SHMs based on absolute VA/PA. These functions are not used and this kind of usage should not be encouraged anyway. Remove these functions.
Signed-off-by: Andrew Davis afd@ti.com Reviewed-by: Sumit Garg sumit.garg@linaro.org --- drivers/tee/tee_shm.c | 50 ----------------------------------------- include/linux/tee_drv.h | 18 --------------- 2 files changed, 68 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index f31e29e8f1cac..b0c6d553d3a70 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -414,56 +414,6 @@ void tee_shm_free(struct tee_shm *shm) } EXPORT_SYMBOL_GPL(tee_shm_free);
-/** - * tee_shm_va2pa() - Get physical address of a virtual address - * @shm: Shared memory handle - * @va: Virtual address to tranlsate - * @pa: Returned physical address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa) -{ - if (!shm->kaddr) - return -EINVAL; - /* Check that we're in the range of the shm */ - if ((char *)va < (char *)shm->kaddr) - return -EINVAL; - if ((char *)va >= ((char *)shm->kaddr + shm->size)) - return -EINVAL; - - return tee_shm_get_pa( - shm, (unsigned long)va - (unsigned long)shm->kaddr, pa); -} -EXPORT_SYMBOL_GPL(tee_shm_va2pa); - -/** - * tee_shm_pa2va() - Get virtual address of a physical address - * @shm: Shared memory handle - * @pa: Physical address to tranlsate - * @va: Returned virtual address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va) -{ - if (!shm->kaddr) - return -EINVAL; - /* Check that we're in the range of the shm */ - if (pa < shm->paddr) - return -EINVAL; - if (pa >= (shm->paddr + shm->size)) - return -EINVAL; - - if (va) { - void *v = tee_shm_get_va(shm, pa - shm->paddr); - - if (IS_ERR(v)) - return PTR_ERR(v); - *va = v; - } - return 0; -} -EXPORT_SYMBOL_GPL(tee_shm_pa2va); - /** * tee_shm_get_va() - Get virtual address of a shared memory plus an offset * @shm: Shared memory handle diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 911cad324acc7..17eb1c5205d34 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -298,24 +298,6 @@ void tee_shm_free(struct tee_shm *shm); */ void tee_shm_put(struct tee_shm *shm);
-/** - * tee_shm_va2pa() - Get physical address of a virtual address - * @shm: Shared memory handle - * @va: Virtual address to tranlsate - * @pa: Returned physical address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa); - -/** - * tee_shm_pa2va() - Get virtual address of a physical address - * @shm: Shared memory handle - * @pa: Physical address to tranlsate - * @va: Returned virtual address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va); - /** * tee_shm_get_va() - Get virtual address of a shared memory plus an offset * @shm: Shared memory handle
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
Signed-off-by: Andrew Davis afd@ti.com ---
Changes from v1: - Removed flags return from tee_ioctl_shm_alloc()
drivers/tee/tee_core.c | 2 -- include/uapi/linux/tee.h | 4 ---- 2 files changed, 6 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 8aa1a4836b92f..af0f7c603fa46 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -302,7 +302,6 @@ static int tee_ioctl_shm_alloc(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id; - data.flags = shm->flags; data.size = shm->size;
if (copy_to_user(udata, &data, sizeof(data))) @@ -339,7 +338,6 @@ tee_ioctl_shm_register(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id; - data.flags = shm->flags; data.length = shm->size;
if (copy_to_user(udata, &data, sizeof(data))) diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 25a6c534beb1b..23e57164693c4 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -42,10 +42,6 @@ #define TEE_IOC_MAGIC 0xa4 #define TEE_IOC_BASE 0
-/* Flags relating to shared memory */ -#define TEE_IOCTL_SHM_MAPPED 0x1 /* memory mapped in normal world */ -#define TEE_IOCTL_SHM_DMA_BUF 0x2 /* dma-buf handle on shared memory */ - #define TEE_MAX_ARG_SIZE 1024
#define TEE_GEN_CAP_GP (1 << 0)/* GlobalPlatform compliant TEE */
On Mon, 25 Apr 2022 at 19:46, Andrew Davis afd@ti.com wrote:
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
Signed-off-by: Andrew Davis afd@ti.com
Changes from v1:
- Removed flags return from tee_ioctl_shm_alloc()
Acked-by: Sumit Garg sumit.garg@linaro.org
-Sumit
drivers/tee/tee_core.c | 2 -- include/uapi/linux/tee.h | 4 ---- 2 files changed, 6 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 8aa1a4836b92f..af0f7c603fa46 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -302,7 +302,6 @@ static int tee_ioctl_shm_alloc(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id;
data.flags = shm->flags; data.size = shm->size; if (copy_to_user(udata, &data, sizeof(data)))@@ -339,7 +338,6 @@ tee_ioctl_shm_register(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id;
data.flags = shm->flags; data.length = shm->size; if (copy_to_user(udata, &data, sizeof(data)))diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 25a6c534beb1b..23e57164693c4 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -42,10 +42,6 @@ #define TEE_IOC_MAGIC 0xa4 #define TEE_IOC_BASE 0
-/* Flags relating to shared memory */ -#define TEE_IOCTL_SHM_MAPPED 0x1 /* memory mapped in normal world */ -#define TEE_IOCTL_SHM_DMA_BUF 0x2 /* dma-buf handle on shared memory */
#define TEE_MAX_ARG_SIZE 1024
#define TEE_GEN_CAP_GP (1 << 0)/* GlobalPlatform compliant TEE */
2.17.1
On Mon, Apr 25, 2022 at 4:18 PM Sumit Garg sumit.garg@linaro.org wrote:
On Mon, 25 Apr 2022 at 19:46, Andrew Davis afd@ti.com wrote:
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
Signed-off-by: Andrew Davis afd@ti.com
Changes from v1:
- Removed flags return from tee_ioctl_shm_alloc()
Acked-by: Sumit Garg sumit.garg@linaro.org
-Sumit
drivers/tee/tee_core.c | 2 -- include/uapi/linux/tee.h | 4 ---- 2 files changed, 6 deletions(-)
Looks good to me, I'm picking up this.
Thanks, Jens
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 8aa1a4836b92f..af0f7c603fa46 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -302,7 +302,6 @@ static int tee_ioctl_shm_alloc(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id;
data.flags = shm->flags; data.size = shm->size; if (copy_to_user(udata, &data, sizeof(data)))@@ -339,7 +338,6 @@ tee_ioctl_shm_register(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id;
data.flags = shm->flags; data.length = shm->size; if (copy_to_user(udata, &data, sizeof(data)))diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 25a6c534beb1b..23e57164693c4 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -42,10 +42,6 @@ #define TEE_IOC_MAGIC 0xa4 #define TEE_IOC_BASE 0
-/* Flags relating to shared memory */ -#define TEE_IOCTL_SHM_MAPPED 0x1 /* memory mapped in normal world */ -#define TEE_IOCTL_SHM_DMA_BUF 0x2 /* dma-buf handle on shared memory */
#define TEE_MAX_ARG_SIZE 1024
#define TEE_GEN_CAP_GP (1 << 0)/* GlobalPlatform compliant TEE */
2.17.1
On Mon, Apr 25, 2022 at 09:16:17AM -0500, Andrew Davis wrote:
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
This change has broken build of strace's test suite against the latest kernel headers[1]; the usage prety much shows up in the Debian's code search[2].
[1] https://github.com/strace/strace/runs/6794205205?check_suite_focus=true#step... [2] https://codesearch.debian.net/search?q=TEE_IOCTL_SHM_MAPPED+package%3A%5CQst...
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
On 6/8/22 8:52 AM, Eugene Syromiatnikov wrote:
On Mon, Apr 25, 2022 at 09:16:17AM -0500, Andrew Davis wrote:
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
This change has broken build of strace's test suite against the latest kernel headers[1]; the usage prety much shows up in the Debian's code search[2].
[1] https://github.com/strace/strace/runs/6794205205?check_suite_focus=true#step... [2] https://codesearch.debian.net/search?q=TEE_IOCTL_SHM_MAPPED+package%3A%5CQst...
Thanks for the headsup, I've sent a patch to fix strace tests.
Andrew
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
On Mon, Apr 25, 2022 at 4:16 PM Andrew Davis afd@ti.com wrote:
We should not need to index into SHMs based on absolute VA/PA. These functions are not used and this kind of usage should not be encouraged anyway. Remove these functions.
Signed-off-by: Andrew Davis afd@ti.com Reviewed-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/tee_shm.c | 50 ----------------------------------------- include/linux/tee_drv.h | 18 --------------- 2 files changed, 68 deletions(-)
Looks good to me, I'm picking up this.
Thanks, Jens
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index f31e29e8f1cac..b0c6d553d3a70 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -414,56 +414,6 @@ void tee_shm_free(struct tee_shm *shm) } EXPORT_SYMBOL_GPL(tee_shm_free);
-/**
- tee_shm_va2pa() - Get physical address of a virtual address
- @shm: Shared memory handle
- @va: Virtual address to tranlsate
- @pa: Returned physical address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa) -{
if (!shm->kaddr)return -EINVAL;/* Check that we're in the range of the shm */if ((char *)va < (char *)shm->kaddr)return -EINVAL;if ((char *)va >= ((char *)shm->kaddr + shm->size))return -EINVAL;return tee_shm_get_pa(shm, (unsigned long)va - (unsigned long)shm->kaddr, pa);-} -EXPORT_SYMBOL_GPL(tee_shm_va2pa);
-/**
- tee_shm_pa2va() - Get virtual address of a physical address
- @shm: Shared memory handle
- @pa: Physical address to tranlsate
- @va: Returned virtual address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va) -{
if (!shm->kaddr)return -EINVAL;/* Check that we're in the range of the shm */if (pa < shm->paddr)return -EINVAL;if (pa >= (shm->paddr + shm->size))return -EINVAL;if (va) {void *v = tee_shm_get_va(shm, pa - shm->paddr);if (IS_ERR(v))return PTR_ERR(v);*va = v;}return 0;-} -EXPORT_SYMBOL_GPL(tee_shm_pa2va);
/**
- tee_shm_get_va() - Get virtual address of a shared memory plus an offset
- @shm: Shared memory handle
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 911cad324acc7..17eb1c5205d34 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -298,24 +298,6 @@ void tee_shm_free(struct tee_shm *shm); */ void tee_shm_put(struct tee_shm *shm);
-/**
- tee_shm_va2pa() - Get physical address of a virtual address
- @shm: Shared memory handle
- @va: Virtual address to tranlsate
- @pa: Returned physical address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa);
-/**
- tee_shm_pa2va() - Get virtual address of a physical address
- @shm: Shared memory handle
- @pa: Physical address to tranlsate
- @va: Returned virtual address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va);
/**
- tee_shm_get_va() - Get virtual address of a shared memory plus an offset
- @shm: Shared memory handle
-- 2.17.1
op-tee@lists.trustedfirmware.org
-
Andrew Davis -
Eugene Syromiatnikov -
Jens Wiklander -
Sumit Garg