[PATCH v4 2/5] cmd: optee_rpmb: close tee session

[Igor Opaniuk]

Hi Ilias,

On Thu, Apr 4, 2024 at 10:54 AM Ilias Apalodimas <
ilias.apalodimas at linaro.org> wrote:

> Hi Igor,
>
> On Thu, 4 Apr 2024 at 11:40, Ilias Apalodimas
> <ilias.apalodimas at 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.opaniuk at 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.opaniuk at gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>