[PATCH v4 2/5] cmd: optee_rpmb: close tee session
Igor Opaniuk
igor.opaniuk at gmail.com
Thu Apr 4 12:58:26 CEST 2024
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>
More information about the U-Boot
mailing list