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

[Igor Opaniuk]

HI Ilias

On Thu, Apr 4, 2024 at 12:15 PM Igor Opaniuk <igor.opaniuk at gmail.com> wrote:

> Hi Ilias,
>
> On Thu, Apr 4, 2024 at 10:40 AM 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
>>
> I usually rely on automatic CC-list creation by patman. Looks like there
> is no
> entry in MAINTAINERS for this specific file, that's why only Tom was added.
> I'll send a patch for that if you don't mind.
>

I've added all orphaned tee-related files to MAINTAINERS in [1], so I patman
should  generate the CC list correctly next time.

[1]
https://lore.kernel.org/u-boot/20240404105248.3241506-1-igor.opaniuk@gmail.com/T/#u


>
>
>> 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?
>>
> That's correct, but avb_ta_open_session() wrapper calls
> both tee_find_device()/
> tee_open_session(), and tee_shm_alloc() expects valid tee device as param,
> which
> is initialized by tee_find_device(). My intention was to address the only
> one particular issue
> that was catched by CI and explained in [1] instead of doing overhaul of
> the whole optee_rpmb cmd implementation.
>
>
>> >
>> >       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
>>
>
> [1]
> https://lore.kernel.org/all/20240302220108.720637-5-igor.opaniuk@gmail.com/T/
>
> --
> 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>
>


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