Re: [PATCH 2/2] tee: optee: don't enumerate services if there ain't any

2023-12-19 Thread Tom Rini 
On Wed, Nov 29, 2023 at 01:37:53PM +0100, Etienne Carriere wrote:

> Change optee driver service enumeration to not enumerate (and
> allocate a zero sized shared memory buffer) when OP-TEE
> reports that there is no service to enumerate.
> 
> This change fixes an existing issue that occurs when the such zero
> sized shared memory buffer allocated from malloc() has a physical
> address of offset 0 of a physical 4kB page. In such case, OP-TEE
> secure world refuses to register the zero-sized shared memory
> area and makes U-Boot optee service enumeration to fail.
> 
> Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services")
> Signed-off-by: Etienne Carriere 
> Reviewed-by: Patrice Chotard 
> Reviewed-by: Jens Wiklander 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 2/2] tee: optee: don't enumerate services if there ain't any

2023-12-05 Thread Jens Wiklander 
On Wed, Nov 29, 2023 at 1:37 PM Etienne Carriere
 wrote:
>
> Change optee driver service enumeration to not enumerate (and
> allocate a zero sized shared memory buffer) when OP-TEE
> reports that there is no service to enumerate.
>
> This change fixes an existing issue that occurs when the such zero
> sized shared memory buffer allocated from malloc() has a physical
> address of offset 0 of a physical 4kB page. In such case, OP-TEE
> secure world refuses to register the zero-sized shared memory
> area and makes U-Boot optee service enumeration to fail.
>
> Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services")
> Signed-off-by: Etienne Carriere 
> ---
>  drivers/tee/optee/core.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Jens Wiklander 

Thanks,
Jens

>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 5308dd58ce..47f845cffe 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -139,6 +139,11 @@ static int enum_services(struct udevice *dev, struct 
> tee_shm **shm, size_t *coun
> if (ret)
> return ret;
>
> +   if (!shm_size) {
> +   *count = 0;
> +   return 0;
> +   }
> +
> ret = tee_shm_alloc(dev, shm_size, 0, shm);
> if (ret) {
> dev_err(dev, "Failed to allocated shared memory: %d\n", ret);
> @@ -185,14 +190,15 @@ static int bind_service_drivers(struct udevice *dev)
>
> ret = enum_services(dev, _list, _count, tee_sess,
> PTA_CMD_GET_DEVICES);
> -   if (!ret)
> +   if (!ret && service_count)
> ret = bind_service_list(dev, service_list, service_count);
>
> tee_shm_free(service_list);
> +   service_list = NULL;
>
> ret2 = enum_services(dev, _list, _count, tee_sess,
>  PTA_CMD_GET_DEVICES_SUPP);
> -   if (!ret2)
> +   if (!ret2 && service_count)
> ret2 = bind_service_list(dev, service_list, service_count);
>
> tee_shm_free(service_list);
> --
> 2.25.1
>

Re: [PATCH 2/2] tee: optee: don't enumerate services if there ain't any

2023-12-01 Thread Patrice CHOTARD 



On 11/29/23 13:37, Etienne Carriere wrote:
> Change optee driver service enumeration to not enumerate (and
> allocate a zero sized shared memory buffer) when OP-TEE
> reports that there is no service to enumerate.
> 
> This change fixes an existing issue that occurs when the such zero
> sized shared memory buffer allocated from malloc() has a physical
> address of offset 0 of a physical 4kB page. In such case, OP-TEE
> secure world refuses to register the zero-sized shared memory
> area and makes U-Boot optee service enumeration to fail.
> 
> Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services")
> Signed-off-by: Etienne Carriere 
> ---
>  drivers/tee/optee/core.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 5308dd58ce..47f845cffe 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -139,6 +139,11 @@ static int enum_services(struct udevice *dev, struct 
> tee_shm **shm, size_t *coun
>   if (ret)
>   return ret;
>  
> + if (!shm_size) {
> + *count = 0;
> + return 0;
> + }
> +
>   ret = tee_shm_alloc(dev, shm_size, 0, shm);
>   if (ret) {
>   dev_err(dev, "Failed to allocated shared memory: %d\n", ret);
> @@ -185,14 +190,15 @@ static int bind_service_drivers(struct udevice *dev)
>  
>   ret = enum_services(dev, _list, _count, tee_sess,
>   PTA_CMD_GET_DEVICES);
> - if (!ret)
> + if (!ret && service_count)
>   ret = bind_service_list(dev, service_list, service_count);
>  
>   tee_shm_free(service_list);
> + service_list = NULL;
>  
>   ret2 = enum_services(dev, _list, _count, tee_sess,
>PTA_CMD_GET_DEVICES_SUPP);
> - if (!ret2)
> + if (!ret2 && service_count)
>   ret2 = bind_service_list(dev, service_list, service_count);
>  
>   tee_shm_free(service_list);
Reviewed-by: Patrice Chotard 

Thanks
Patrice

[PATCH 2/2] tee: optee: don't enumerate services if there ain't any

2023-11-29 Thread Etienne Carriere 
Change optee driver service enumeration to not enumerate (and
allocate a zero sized shared memory buffer) when OP-TEE
reports that there is no service to enumerate.

This change fixes an existing issue that occurs when the such zero
sized shared memory buffer allocated from malloc() has a physical
address of offset 0 of a physical 4kB page. In such case, OP-TEE
secure world refuses to register the zero-sized shared memory
area and makes U-Boot optee service enumeration to fail.

Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services")
Signed-off-by: Etienne Carriere 
---
 drivers/tee/optee/core.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 5308dd58ce..47f845cffe 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -139,6 +139,11 @@ static int enum_services(struct udevice *dev, struct 
tee_shm **shm, size_t *coun
if (ret)
return ret;
 
+   if (!shm_size) {
+   *count = 0;
+   return 0;
+   }
+
ret = tee_shm_alloc(dev, shm_size, 0, shm);
if (ret) {
dev_err(dev, "Failed to allocated shared memory: %d\n", ret);
@@ -185,14 +190,15 @@ static int bind_service_drivers(struct udevice *dev)
 
ret = enum_services(dev, _list, _count, tee_sess,
PTA_CMD_GET_DEVICES);
-   if (!ret)
+   if (!ret && service_count)
ret = bind_service_list(dev, service_list, service_count);
 
tee_shm_free(service_list);
+   service_list = NULL;
 
ret2 = enum_services(dev, _list, _count, tee_sess,
 PTA_CMD_GET_DEVICES_SUPP);
-   if (!ret2)
+   if (!ret2 && service_count)
ret2 = bind_service_list(dev, service_list, service_count);
 
tee_shm_free(service_list);
-- 
2.25.1