diff mbox series

tee: optee: fix device enumeration error handling

Message ID 20191106155538.5618-1-jens.wiklander@linaro.org
State Accepted
Commit 03212e347f9443e524d6383c6806ac08295c1fb0
Headers show
Series tee: optee: fix device enumeration error handling | expand

Commit Message

Jens Wiklander Nov. 6, 2019, 3:55 p.m. UTC
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(-)

-- 
2.17.1

Comments

Sumit Garg Nov. 7, 2019, 5:11 a.m. UTC | #1
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

>
diff mbox series

Patch

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;