params_from_user() acquires tee_shm references for MEMREF parameters and expects the caller to release those references with tee_shm_put() during cleanup.

tee_ioctl_open_session(), tee_ioctl_invoke(), and tee_ioctl_object_invoke() all do this, but tee_ioctl_supp_recv() only frees the parameter array and does not drop any acquired shared-memory references.

Fix this by using a common helper to release MEMREF references before freeing the parameter array, and apply it to tee_ioctl_supp_recv() as well.

Since supp_recv backends may update num_params, preserve the original allocated parameter count for cleanup.

Signed-off-by: Qihang q.h.hack.winter@gmail.com --- drivers/tee/tee_core.c | 49 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index ef9642d72672..adad1ea8e31b 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 params_free_decref(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) + 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); - } - + params_free_decref(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); - } + params_free_decref(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); - } + params_free_decref(params, arg.num_params); return rc; }

@@ -822,6 +814,7 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx, struct tee_iocl_supp_recv_arg __user *uarg; struct tee_param *params; u32 num_params; + u32 alloc_num_params; u32 func;

if (!ctx->teedev->desc->ops->supp_recv) @@ -838,6 +831,8 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx, if (get_user(num_params, &uarg->num_params)) return -EFAULT;

+ alloc_num_params = num_params; + if (size_add(sizeof(*uarg), TEE_IOCTL_PARAM_SIZE(num_params)) != buf.buf_len) return -EINVAL;

@@ -861,7 +856,7 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx,

rc = params_to_supp(ctx, uarg->params, num_params, params); out: - kfree(params); + params_free_decref(params, alloc_num_params); return rc; }

Hi Sumit,

Thanks, that makes sense.

I had been conservative about the possibility of num_params being updated by supp_recv backends, but looking again at the current paths, I agree that alloc_num_params is unnecessary here.

I'll drop that, rename the helper to free_params(), and send a v2.

Thanks, Qihang

On Fri, May 1, 2026 at 10:32 PM Sumit Garg sumit.garg@kernel.org wrote:

On Wed, Apr 29, 2026 at 07:32:19PM +0800, Qihang wrote:

...

params_from_user() acquires tee_shm references for MEMREF parameters and expects the caller to release those references with tee_shm_put() during cleanup.

tee_ioctl_open_session(), tee_ioctl_invoke(), and tee_ioctl_object_invoke() all do this, but tee_ioctl_supp_recv() only frees the parameter array and does not drop any acquired shared-memory references.

Fix this by using a common helper to release MEMREF references before freeing the parameter array, and apply it to tee_ioctl_supp_recv() as well.

Since supp_recv backends may update num_params, preserve the original allocated parameter count for cleanup.

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

drivers/tee/tee_core.c | 49 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index ef9642d72672..adad1ea8e31b 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 params_free_decref(struct tee_param *params, size_t num_params)

I would rather rename this API as free_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)
  

•                 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);
  

• }
  

• params_free_decref(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);
  

• }
  

• params_free_decref(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);
  

• }
  

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

}

@@ -822,6 +814,7 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx, struct tee_iocl_supp_recv_arg __user *uarg; struct tee_param *params; u32 num_params;

• u32 alloc_num_params;
  u32 func;
  
  if (!ctx->teedev->desc->ops->supp_recv)
  

@@ -838,6 +831,8 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx, if (get_user(num_params, &uarg->num_params)) return -EFAULT;

• alloc_num_params = num_params;
  

Why is this needed? Shouldn't the updated num_params will point to size of params array?

-Sumit

...

• if (size_add(sizeof(*uarg), TEE_IOCTL_PARAM_SIZE(num_params)) != buf.buf_len)
          return -EINVAL;
  
  

@@ -861,7 +856,7 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx,

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

out:

• kfree(params);
  

• params_free_decref(params, alloc_num_params);
  return rc;
  

}

-- 2.39.5 (Apple Git-154)

Hi Jens, Sumit,

While reworking this, I noticed that there may be two slightly different cleanup issues involved here.

One appears to be in tee_ioctl_supp_recv() itself. Another may be in a qcomtee-specific error path where MEMREF references can already have been acquired before the backend rejects the parameters.

I am not entirely sure where you would prefer the boundary between the core fix and any backend-specific fix to be.

Would you prefer that I keep the current patch strictly focused on the tee_ioctl_supp_recv() cleanup path first, and handle any qcomtee-side issue separately if needed?

Thanks, Qihang

On Tue, May 5, 2026 at 11:30 PM Qihang q.h.hack.winter@gmail.com wrote:

params_from_user() acquires tee_shm references for MEMREF parameters and expects the caller to release those references with tee_shm_put() during cleanup.

tee_ioctl_open_session(), tee_ioctl_invoke(), and tee_ioctl_object_invoke() all do this, but tee_ioctl_supp_recv() only frees the parameter array and does not drop any acquired shared-memory references.

Fix this by using a common helper to release MEMREF references before freeing the parameter array, and apply it to tee_ioctl_supp_recv() as well.

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

v2:

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

drivers/tee/tee_core.c | 46 +++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index ef9642d72672..8cdf2ec7e74f 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)
  

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

}

@@ -861,7 +853,7 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx,

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

out:

•   kfree(params);
  

•   free_params(params, num_params);
    return rc;
  

}

-- 2.39.5 (Apple Git-154)

Hi Qihang,

On Tue, May 5, 2026 at 5:30 PM Qihang q.h.hack.winter@gmail.com wrote:

params_from_user() acquires tee_shm references for MEMREF parameters and expects the caller to release those references with tee_shm_put() during cleanup.

tee_ioctl_open_session(), tee_ioctl_invoke(), and tee_ioctl_object_invoke() all do this, but tee_ioctl_supp_recv() only frees the parameter array and does not drop any acquired shared-memory references.

Fix this by using a common helper to release MEMREF references before freeing the parameter array, and apply it to tee_ioctl_supp_recv() as well.

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

v2:

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

drivers/tee/tee_core.c | 46 +++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index ef9642d72672..8cdf2ec7e74f 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)
  

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

}

@@ -861,7 +853,7 @@ static int tee_ioctl_supp_recv(struct tee_context *ctx,

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

out:

•   kfree(params);
  

•   free_params(params, num_params);
  

This doesn't work. The supp_recv() callback is special compared to open_session(), invoke_func(), and object_invoke_func() callbacks. supp_recv() replaces the supplied parameters with other parameters. Eventually inserted memrefs are reference-counted separately, and adding it here too would only add unnecessary updates (supp_recv() would need to increase them, only so they can be decreased here). However, I think a comment before the call to supp_recv() to remind us of that would be nice.

However, there's a bug that needs to be fixed if params_from_user() fails. One way of dealing with that is: rc = params_from_user(ctx, params, num_params, uarg->params); if (rc) { free_params(params, num_params); return rc; }

and then keep the end of tee_ioctl_supp_recv() unchanged.

Cheers, Jens

    return rc;

}

-- 2.39.5 (Apple Git-154)

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