Hi Ilias, On Thu, Apr 4, 2024 at 10:54 AM Ilias Apalodimas < ilias.apalodi...@linaro.org> wrote:
> Hi Igor, > > On Thu, 4 Apr 2024 at 11:40, Ilias Apalodimas > <ilias.apalodi...@linaro.org> wrote: > > > > Hi Igor, > > > > I was about to apply the series, but noticed neither me or Jens were > cc'ed > > on this. Adding Jens to the party > > > > On Thu, Apr 04, 2024 at 10:13:49AM +0200, Igor Opaniuk wrote: > > > Add calls for closing tee session after every read/write operation. > > > > What the patch is pretty obvious, but I am missing an explanation of why > > this is needed > > > > > > > > Signed-off-by: Igor Opaniuk <igor.opan...@gmail.com> > > > --- > > > > > > (no changes since v1) > > > > > > cmd/optee_rpmb.c | 23 +++++++++++++++++------ > > > 1 file changed, 17 insertions(+), 6 deletions(-) > > > > > > diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c > > > index e0e44bbed04..b3cafd92410 100644 > > > --- a/cmd/optee_rpmb.c > > > +++ b/cmd/optee_rpmb.c > > > @@ -87,8 +87,10 @@ static int read_persistent_value(const char *name, > > > > > > rc = tee_shm_alloc(tee, name_size, > > > TEE_SHM_ALLOC, &shm_name); > > > - if (rc) > > > - return -ENOMEM; > > > + if (rc) { > > > + rc = -ENOMEM; > > > + goto close_session; > > > + } > > > > I don't think you need the session to be opened to allocate memory. > > So instead of of doing this, why don't we just move the alloc call before > > opening the session? > > Looking at it again, we do need tee != NULL, but I think it's cleaner > to add something like > if (!dev) > return -ENODEV > to __tee_shm_add() instead. > Do you mind if I address that in a separate patch series, as tbh I don't want to add additional patches to the existing series? > > Cheers > /Ilias > > > > > > > > rc = tee_shm_alloc(tee, buffer_size, > > > TEE_SHM_ALLOC, &shm_buf); > > > @@ -125,6 +127,9 @@ out: > > > tee_shm_free(shm_buf); > > > free_name: > > > tee_shm_free(shm_name); > > > +close_session: > > > + tee_close_session(tee, session); > > > + tee = NULL; > > > > > > return rc; > > > } > > > @@ -139,17 +144,20 @@ static int write_persistent_value(const char > *name, > > > struct tee_param param[2]; > > > size_t name_size = strlen(name) + 1; > > > > > > + if (!value_size) > > > + return -EINVAL; > > > + > > > if (!tee) { > > > if (avb_ta_open_session()) > > > return -ENODEV; > > > } > > > - if (!value_size) > > > - return -EINVAL; > > > > > > rc = tee_shm_alloc(tee, name_size, > > > TEE_SHM_ALLOC, &shm_name); > > > - if (rc) > > > - return -ENOMEM; > > > + if (rc) { > > > + rc = -ENOMEM; > > > + goto close_session; > > > + } > > > > ditto > > > > > > > > rc = tee_shm_alloc(tee, value_size, > > > TEE_SHM_ALLOC, &shm_buf); > > > @@ -178,6 +186,9 @@ out: > > > tee_shm_free(shm_buf); > > > free_name: > > > tee_shm_free(shm_name); > > > +close_session: > > > + tee_close_session(tee, session); > > > + tee = NULL; > > > > > > return rc; > > > } > > > -- > > > 2.34.1 > > > > > > > Thanks > > /Ilias > -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>