* [PATCH v2] efi_loader: Extra checks while opening an OPTEE session
@ 2020-12-23 11:25 Ilias Apalodimas
2020-12-23 11:27 ` Ilias Apalodimas
0 siblings, 1 reply; 3+ messages in thread
From: Ilias Apalodimas @ 2020-12-23 11:25 UTC (permalink / raw)
To: u-boot
When opening an OP-TEE session we need to check the internal return
value of OP-TEE call arguments as well the return code of the
function itself.
The code was also ignoring to close the OP-TEE session in case the
shared memory registration failed.
Fixes: f042e47e8fb43 ("efi_loader: Implement EFI variable handling via OP-TEE")
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
changes since v1:
- add a goto tag and use it on fails
lib/efi_loader/efi_variable_tee.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
index be6f3dfad469..b8808fdecad3 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -36,20 +36,29 @@ static int get_connection(struct mm_connection *conn)
static const struct tee_optee_ta_uuid uuid = PTA_STMM_UUID;
struct udevice *tee = NULL;
struct tee_open_session_arg arg;
- int rc;
+ int rc = -ENODEV;
tee = tee_find_device(tee, NULL, NULL, NULL);
if (!tee)
- return -ENODEV;
+ goto out;
memset(&arg, 0, sizeof(arg));
tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
rc = tee_open_session(tee, &arg, 0, NULL);
- if (!rc) {
- conn->tee = tee;
- conn->session = arg.session;
+ if (rc)
+ goto out;
+
+ /* Check the internal OP-TEE result */
+ if (arg.ret != TEE_SUCCESS) {
+ rc = -EIO;
+ goto out;
}
+ conn->tee = tee;
+ conn->session = arg.session;
+
+ return 0;
+out:
return rc;
}
@@ -88,6 +97,7 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {
log_err("Unable to register shared memory\n");
+ tee_close_session(conn.tee, conn.session);
return EFI_UNSUPPORTED;
}
--
2.30.0.rc1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v2] efi_loader: Extra checks while opening an OPTEE session
2020-12-23 11:25 [PATCH v2] efi_loader: Extra checks while opening an OPTEE session Ilias Apalodimas
@ 2020-12-23 11:27 ` Ilias Apalodimas
2020-12-23 11:28 ` Ilias Apalodimas
0 siblings, 1 reply; 3+ messages in thread
From: Ilias Apalodimas @ 2020-12-23 11:27 UTC (permalink / raw)
To: u-boot
Heinrich I found a slightly better way to do it and free teh session
on errors, so we dont have to check it.
I'll send a v3
Cheers
/Ilias
On Wed, 23 Dec 2020 at 13:25, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> When opening an OP-TEE session we need to check the internal return
> value of OP-TEE call arguments as well the return code of the
> function itself.
> The code was also ignoring to close the OP-TEE session in case the
> shared memory registration failed.
>
> Fixes: f042e47e8fb43 ("efi_loader: Implement EFI variable handling via OP-TEE")
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> changes since v1:
> - add a goto tag and use it on fails
>
> lib/efi_loader/efi_variable_tee.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index be6f3dfad469..b8808fdecad3 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -36,20 +36,29 @@ static int get_connection(struct mm_connection *conn)
> static const struct tee_optee_ta_uuid uuid = PTA_STMM_UUID;
> struct udevice *tee = NULL;
> struct tee_open_session_arg arg;
> - int rc;
> + int rc = -ENODEV;
>
> tee = tee_find_device(tee, NULL, NULL, NULL);
> if (!tee)
> - return -ENODEV;
> + goto out;
>
> memset(&arg, 0, sizeof(arg));
> tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
> rc = tee_open_session(tee, &arg, 0, NULL);
> - if (!rc) {
> - conn->tee = tee;
> - conn->session = arg.session;
> + if (rc)
> + goto out;
> +
> + /* Check the internal OP-TEE result */
> + if (arg.ret != TEE_SUCCESS) {
> + rc = -EIO;
> + goto out;
> }
>
> + conn->tee = tee;
> + conn->session = arg.session;
> +
> + return 0;
> +out:
> return rc;
> }
>
> @@ -88,6 +97,7 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
>
> if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {
> log_err("Unable to register shared memory\n");
> + tee_close_session(conn.tee, conn.session);
> return EFI_UNSUPPORTED;
> }
>
> --
> 2.30.0.rc1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v2] efi_loader: Extra checks while opening an OPTEE session
2020-12-23 11:27 ` Ilias Apalodimas
@ 2020-12-23 11:28 ` Ilias Apalodimas
0 siblings, 0 replies; 3+ messages in thread
From: Ilias Apalodimas @ 2020-12-23 11:28 UTC (permalink / raw)
To: u-boot
Apologies for the noise.
This version should be fine! You can pick it up if you like it.
Cheers
/Ilias
On Wed, 23 Dec 2020 at 13:27, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Heinrich I found a slightly better way to do it and free teh session
> on errors, so we dont have to check it.
> I'll send a v3
>
> Cheers
> /Ilias
>
>
> On Wed, 23 Dec 2020 at 13:25, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > When opening an OP-TEE session we need to check the internal return
> > value of OP-TEE call arguments as well the return code of the
> > function itself.
> > The code was also ignoring to close the OP-TEE session in case the
> > shared memory registration failed.
> >
> > Fixes: f042e47e8fb43 ("efi_loader: Implement EFI variable handling via OP-TEE")
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> > changes since v1:
> > - add a goto tag and use it on fails
> >
> > lib/efi_loader/efi_variable_tee.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > index be6f3dfad469..b8808fdecad3 100644
> > --- a/lib/efi_loader/efi_variable_tee.c
> > +++ b/lib/efi_loader/efi_variable_tee.c
> > @@ -36,20 +36,29 @@ static int get_connection(struct mm_connection *conn)
> > static const struct tee_optee_ta_uuid uuid = PTA_STMM_UUID;
> > struct udevice *tee = NULL;
> > struct tee_open_session_arg arg;
> > - int rc;
> > + int rc = -ENODEV;
> >
> > tee = tee_find_device(tee, NULL, NULL, NULL);
> > if (!tee)
> > - return -ENODEV;
> > + goto out;
> >
> > memset(&arg, 0, sizeof(arg));
> > tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
> > rc = tee_open_session(tee, &arg, 0, NULL);
> > - if (!rc) {
> > - conn->tee = tee;
> > - conn->session = arg.session;
> > + if (rc)
> > + goto out;
> > +
> > + /* Check the internal OP-TEE result */
> > + if (arg.ret != TEE_SUCCESS) {
> > + rc = -EIO;
> > + goto out;
> > }
> >
> > + conn->tee = tee;
> > + conn->session = arg.session;
> > +
> > + return 0;
> > +out:
> > return rc;
> > }
> >
> > @@ -88,6 +97,7 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
> >
> > if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {
> > log_err("Unable to register shared memory\n");
> > + tee_close_session(conn.tee, conn.session);
> > return EFI_UNSUPPORTED;
> > }
> >
> > --
> > 2.30.0.rc1
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-23 11:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-23 11:25 [PATCH v2] efi_loader: Extra checks while opening an OPTEE session Ilias Apalodimas
2020-12-23 11:27 ` Ilias Apalodimas
2020-12-23 11:28 ` Ilias Apalodimas
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.