2019-11-06 15:57:15

by Jens Wiklander

Subject: [PATCH] tee: optee: fix device enumeration error handling

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 <[email protected]>
---
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

2019-11-07 05:18:15

by Sumit Garg

Subject: Re: [PATCH] tee: optee: fix device enumeration error handling

On Wed, 6 Nov 2019 at 21:25, Jens Wiklander <[email protected]> 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 <[email protected]>
> ---
> drivers/tee/optee/core.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>

Reviewed-by: Sumit Garg <[email protected]>

> 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
>