| Message ID | 20201223112501.58584-1-ilias.apalodimas@linaro.org |
|---|---|
| State | Accepted |
| Commit | 548fb67eef14f83d60310193415ac9d06a3bd286 |
| Headers | show |
| Series | [v2] efi_loader: Extra checks while opening an OPTEE session | expand |
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 >
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 > >
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; }
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(-) -- 2.30.0.rc1