Re: [PATCH v3] tee: fix params_from_user() error path in tee_ioctl_supp_recv

Hi,

On Thu, May 7, 2026 at 11:46 AM Qihang q.h.hack.winter@gmail.com wrote:

params_from_user() may acquire tee_shm references for MEMREF parameters before failing after partially processing the supplied parameter array.

In tee_ioctl_supp_recv(), those references are currently not released on that error path.

Fix this by freeing MEMREF references before returning when params_from_user() fails.

Keep the final cleanup path in tee_ioctl_supp_recv() unchanged since supp_recv() may consume and replace the supplied parameters, unlike the other TEE ioctl callback paths.

Signed-off-by: Qihang q.h.hack.winter@gmail.com

v3:

• only free MEMREF references when params_from_user() fails
• keep tee_ioctl_supp_recv() final cleanup unchanged
• follow Jens' review on supp_recv() parameter ownership semantics

v2:

• rename helper to free_params()
• drop alloc_num_params and use num_params directly

drivers/tee/tee_core.c | 54 ++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index ef9642d72672..2fd3a00b47c7 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -530,6 +530,21 @@ static int params_to_user(struct tee_ioctl_param __user *uparams, return 0; }

+static void free_params(struct tee_param *params, size_t num_params) +{

•   size_t n;
  

•   if (!params)
  

•           return;
  

•   for (n = 0; n < num_params; n++)
  

•           if (tee_param_is_memref(params + n) &&
  

•               params[n].u.memref.shm)
  

Please fold up. It fits on one line.

•                   tee_shm_put(params[n].u.memref.shm);
  

•   kfree(params);
  

+}

static int tee_ioctl_open_session(struct tee_context *ctx, struct tee_ioctl_buf_data __user *ubuf) { @@ -595,16 +610,7 @@ static int tee_ioctl_open_session(struct tee_context *ctx, */ if (rc && have_session && ctx->teedev->desc->ops->close_session) ctx->teedev->desc->ops->close_session(ctx, arg.session);

•   if (params) {
  

•           /* Decrease ref count for all valid shared memory pointers */
  

•           for (n = 0; n < arg.num_params; n++)
  

•                   if (tee_param_is_memref(params + n) &&
  

•                       params[n].u.memref.shm)
  

•                           tee_shm_put(params[n].u.memref.shm);
  

•           kfree(params);
  

•   }
  

•   free_params(params, arg.num_params);
    return rc;
  

}

@@ -657,14 +663,7 @@ static int tee_ioctl_invoke(struct tee_context *ctx, } rc = params_to_user(uparams, arg.num_params, params); out:

•   if (params) {
  

•           /* Decrease ref count for all valid shared memory pointers */
  

•           for (n = 0; n < arg.num_params; n++)
  

•                   if (tee_param_is_memref(params + n) &&
  

•                       params[n].u.memref.shm)
  

•                           tee_shm_put(params[n].u.memref.shm);
  

•           kfree(params);
  

•   }
  

•   free_params(params, arg.num_params);
    return rc;
  

}

@@ -716,14 +715,7 @@ static int tee_ioctl_object_invoke(struct tee_context *ctx, } rc = params_to_user(uparams, arg.num_params, params); out:

•   if (params) {
  

•           /* Decrease ref count for all valid shared memory pointers */
  

•           for (n = 0; n < arg.num_params; n++)
  

•                   if (tee_param_is_memref(params + n) &&
  

•                       params[n].u.memref.shm)
  

•                           tee_shm_put(params[n].u.memref.shm);
  

•           kfree(params);
  

•   }
  

•   free_params(params, arg.num_params);
    return rc;
  

}

@@ -846,9 +838,15 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx, return -ENOMEM;

    rc = params_from_user(ctx, params, num_params, uarg->params);

•   if (rc)
  

•           goto out;
  

•   if (rc) {
  

•           free_params(params, num_params);
  

•           return rc;
  

•   }
  
  

•   /*
  

•    * supp_recv() may consume and replace the supplied parameters, so the
  

•    * final cleanup cannot use free_params() like the other ioctl paths.
  

•    */
    rc = ctx->teedev->desc->ops->supp_recv(ctx, &func, &num_params, params);
    if (rc)
            goto out;
  

-- 2.39.5 (Apple Git-154)

There are a few warnings: AR drivers/base/firmware_loader/built-in.a AR drivers/base/built-in.a drivers/tee/tee_core.c: In function ‘tee_ioctl_open_session’: drivers/tee/tee_core.c:552:16: warning: unused variable ‘n’ [-Wunused-variable] 552 | size_t n; | ^ drivers/tee/tee_core.c: In function ‘tee_ioctl_invoke’: drivers/tee/tee_core.c:622:16: warning: unused variable ‘n’ [-Wunused-variable] 622 | size_t n; | ^ drivers/tee/tee_core.c: In function ‘tee_ioctl_object_invoke’: drivers/tee/tee_core.c:676:16: warning: unused variable ‘n’ [-Wunused-variable] 676 | size_t n; | ^

Cheers, Jens