All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tee: optee: fix device enumeration error handling
@ 2019-11-06 15:55 ` Jens Wiklander
  0 siblings, 0 replies; 4+ messages in thread
From: Jens Wiklander @ 2019-11-06 15:55 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, tee-dev
  Cc: Victor Chong, Jerome Forissier, Sumit Garg, Etienne Carriere,
	Jens Wiklander

Prior to this patch in optee_probe() when optee_enumerate_devices() was
called the struct optee was fully initialized. If
optee_enumerate_devices() returns an error optee_probe() is supposed to
clean up and free the struct optee completely, but will at this late
stage need to call optee_remove() instead. This isn't done and thus
freeing the struct optee prematurely.

With this patch the call to optee_enumerate_devices() is done after
optee_probe() has returned successfully and in case
optee_enumerate_devices() fails everything is cleaned up with a call to
optee_remove().

Fixes: c3fa24af9244 ("tee: optee: add TEE bus device enumeration support")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 drivers/tee/optee/core.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 1854a3db7345..b830e0a87fba 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -643,11 +643,6 @@ static struct optee *optee_probe(struct device_node *np)
 	if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
 		pr_info("dynamic shared memory is enabled\n");
 
-	rc = optee_enumerate_devices();
-	if (rc)
-		goto err;
-
-	pr_info("initialized driver\n");
 	return optee;
 err:
 	if (optee) {
@@ -702,9 +697,10 @@ static struct optee *optee_svc;
 
 static int __init optee_driver_init(void)
 {
-	struct device_node *fw_np;
-	struct device_node *np;
-	struct optee *optee;
+	struct device_node *fw_np = NULL;
+	struct device_node *np = NULL;
+	struct optee *optee = NULL;
+	int rc = 0;
 
 	/* Node is supposed to be below /firmware */
 	fw_np = of_find_node_by_name(NULL, "firmware");
@@ -723,6 +719,14 @@ static int __init optee_driver_init(void)
 	if (IS_ERR(optee))
 		return PTR_ERR(optee);
 
+	rc = optee_enumerate_devices();
+	if (rc) {
+		optee_remove(optee);
+		return rc;
+	}
+
+	pr_info("initialized driver\n");
+
 	optee_svc = optee;
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] tee: optee: fix device enumeration error handling
@ 2019-11-06 15:55 ` Jens Wiklander
  0 siblings, 0 replies; 4+ messages in thread
From: Jens Wiklander @ 2019-11-06 15:55 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, tee-dev
  Cc: Sumit Garg, Victor Chong, Etienne Carriere, Jerome Forissier,
	Jens Wiklander

Prior to this patch in optee_probe() when optee_enumerate_devices() was
called the struct optee was fully initialized. If
optee_enumerate_devices() returns an error optee_probe() is supposed to
clean up and free the struct optee completely, but will at this late
stage need to call optee_remove() instead. This isn't done and thus
freeing the struct optee prematurely.

With this patch the call to optee_enumerate_devices() is done after
optee_probe() has returned successfully and in case
optee_enumerate_devices() fails everything is cleaned up with a call to
optee_remove().

Fixes: c3fa24af9244 ("tee: optee: add TEE bus device enumeration support")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 drivers/tee/optee/core.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 1854a3db7345..b830e0a87fba 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -643,11 +643,6 @@ static struct optee *optee_probe(struct device_node *np)
 	if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
 		pr_info("dynamic shared memory is enabled\n");
 
-	rc = optee_enumerate_devices();
-	if (rc)
-		goto err;
-
-	pr_info("initialized driver\n");
 	return optee;
 err:
 	if (optee) {
@@ -702,9 +697,10 @@ static struct optee *optee_svc;
 
 static int __init optee_driver_init(void)
 {
-	struct device_node *fw_np;
-	struct device_node *np;
-	struct optee *optee;
+	struct device_node *fw_np = NULL;
+	struct device_node *np = NULL;
+	struct optee *optee = NULL;
+	int rc = 0;
 
 	/* Node is supposed to be below /firmware */
 	fw_np = of_find_node_by_name(NULL, "firmware");
@@ -723,6 +719,14 @@ static int __init optee_driver_init(void)
 	if (IS_ERR(optee))
 		return PTR_ERR(optee);
 
+	rc = optee_enumerate_devices();
+	if (rc) {
+		optee_remove(optee);
+		return rc;
+	}
+
+	pr_info("initialized driver\n");
+
 	optee_svc = optee;
 
 	return 0;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] tee: optee: fix device enumeration error handling
  2019-11-06 15:55 ` Jens Wiklander
@ 2019-11-07  5:11   ` Sumit Garg
  -1 siblings, 0 replies; 4+ messages in thread
From: Sumit Garg @ 2019-11-07  5:11 UTC (permalink / raw)
  To: Jens Wiklander
  Cc: Linux Kernel Mailing List, linux-arm-kernel,
	tee-dev @ lists . linaro . org, Victor Chong, Jerome Forissier,
	Etienne Carriere

On Wed, 6 Nov 2019 at 21:25, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Prior to this patch in optee_probe() when optee_enumerate_devices() was
> called the struct optee was fully initialized. If
> optee_enumerate_devices() returns an error optee_probe() is supposed to
> clean up and free the struct optee completely, but will at this late
> stage need to call optee_remove() instead. This isn't done and thus
> freeing the struct optee prematurely.
>
> With this patch the call to optee_enumerate_devices() is done after
> optee_probe() has returned successfully and in case
> optee_enumerate_devices() fails everything is cleaned up with a call to
> optee_remove().
>
> Fixes: c3fa24af9244 ("tee: optee: add TEE bus device enumeration support")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  drivers/tee/optee/core.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 1854a3db7345..b830e0a87fba 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -643,11 +643,6 @@ static struct optee *optee_probe(struct device_node *np)
>         if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
>                 pr_info("dynamic shared memory is enabled\n");
>
> -       rc = optee_enumerate_devices();
> -       if (rc)
> -               goto err;
> -
> -       pr_info("initialized driver\n");
>         return optee;
>  err:
>         if (optee) {
> @@ -702,9 +697,10 @@ static struct optee *optee_svc;
>
>  static int __init optee_driver_init(void)
>  {
> -       struct device_node *fw_np;
> -       struct device_node *np;
> -       struct optee *optee;
> +       struct device_node *fw_np = NULL;
> +       struct device_node *np = NULL;
> +       struct optee *optee = NULL;
> +       int rc = 0;
>
>         /* Node is supposed to be below /firmware */
>         fw_np = of_find_node_by_name(NULL, "firmware");
> @@ -723,6 +719,14 @@ static int __init optee_driver_init(void)
>         if (IS_ERR(optee))
>                 return PTR_ERR(optee);
>
> +       rc = optee_enumerate_devices();
> +       if (rc) {
> +               optee_remove(optee);
> +               return rc;
> +       }
> +
> +       pr_info("initialized driver\n");
> +
>         optee_svc = optee;
>
>         return 0;
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] tee: optee: fix device enumeration error handling
@ 2019-11-07  5:11   ` Sumit Garg
  0 siblings, 0 replies; 4+ messages in thread
From: Sumit Garg @ 2019-11-07  5:11 UTC (permalink / raw)
  To: Jens Wiklander
  Cc: Jerome Forissier, Linux Kernel Mailing List,
	tee-dev @ lists . linaro . org, Victor Chong, Etienne Carriere,
	linux-arm-kernel

On Wed, 6 Nov 2019 at 21:25, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Prior to this patch in optee_probe() when optee_enumerate_devices() was
> called the struct optee was fully initialized. If
> optee_enumerate_devices() returns an error optee_probe() is supposed to
> clean up and free the struct optee completely, but will at this late
> stage need to call optee_remove() instead. This isn't done and thus
> freeing the struct optee prematurely.
>
> With this patch the call to optee_enumerate_devices() is done after
> optee_probe() has returned successfully and in case
> optee_enumerate_devices() fails everything is cleaned up with a call to
> optee_remove().
>
> Fixes: c3fa24af9244 ("tee: optee: add TEE bus device enumeration support")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  drivers/tee/optee/core.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 1854a3db7345..b830e0a87fba 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -643,11 +643,6 @@ static struct optee *optee_probe(struct device_node *np)
>         if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
>                 pr_info("dynamic shared memory is enabled\n");
>
> -       rc = optee_enumerate_devices();
> -       if (rc)
> -               goto err;
> -
> -       pr_info("initialized driver\n");
>         return optee;
>  err:
>         if (optee) {
> @@ -702,9 +697,10 @@ static struct optee *optee_svc;
>
>  static int __init optee_driver_init(void)
>  {
> -       struct device_node *fw_np;
> -       struct device_node *np;
> -       struct optee *optee;
> +       struct device_node *fw_np = NULL;
> +       struct device_node *np = NULL;
> +       struct optee *optee = NULL;
> +       int rc = 0;
>
>         /* Node is supposed to be below /firmware */
>         fw_np = of_find_node_by_name(NULL, "firmware");
> @@ -723,6 +719,14 @@ static int __init optee_driver_init(void)
>         if (IS_ERR(optee))
>                 return PTR_ERR(optee);
>
> +       rc = optee_enumerate_devices();
> +       if (rc) {
> +               optee_remove(optee);
> +               return rc;
> +       }
> +
> +       pr_info("initialized driver\n");
> +
>         optee_svc = optee;
>
>         return 0;
> --
> 2.17.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-07  5:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-06 15:55 [PATCH] tee: optee: fix device enumeration error handling Jens Wiklander
2019-11-06 15:55 ` Jens Wiklander
2019-11-07  5:11 ` Sumit Garg
2019-11-07  5:11   ` Sumit Garg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.