Hi Ilias,
On Thu, Apr 4, 2024 at 10:40 AM Ilias Apalodimas < ilias.apalodimas@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.
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@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...