[REPORT] tee: tee_ioctl_supp_recv() missing tee_shm_put() cleanup for memref params

[REPORT] tee: tee_ioctl_supp_recv() missing tee_shm_put() cleanup for memref params

[Qihang]

Hi,

I would like to report a likely shared-memory reference leak in
drivers/tee/tee_core.c, in tee_ioctl_supp_recv().

The issue is that tee_ioctl_supp_recv() calls params_from_user(), which
acquires references for MEMREF-type parameters via tee_shm_get_from_id(),
but the cleanup path only frees the params array and does not drop the
acquired shm references.

Relevant code in tee_ioctl_supp_recv():

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

  rc = ctx->teedev->desc->ops->supp_recv(ctx, &func, &num_params, params);
  if (rc)
          goto out;
  ...
out:
  kfree(params);
  return rc;

By contrast, other callers of params_from_user() in tee_core.c do release
MEMREF references on cleanup, for example:
- tee_ioctl_open_session()
- tee_ioctl_invoke()
- tee_ioctl_object_invoke()

The helper comment in param_from_user_memref() also states that the caller
is responsible for calling tee_shm_put() on all resolved pointers.

Reachability / trigger paths:

1. qcomtee backend:
   qcomtee registers .supp_recv = qcomtee_supp_recv on a non-privileged
   tee device path. qcomtee_supp_recv() rejects any non-meta parameter
   after the first one:
     for (i = 1; i < *num_params; i++)
             if (params[i].attr)
                     return -EINVAL;
   This allows a path where params_from_user() has already acquired a shm
   reference for a MEMREF parameter, then qcomtee_supp_recv() returns an
   error, and tee_ioctl_supp_recv() exits without tee_shm_put().

2. Partial failure before backend callback:
   if params_from_user() successfully resolves an earlier MEMREF parameter
   and then fails on a later parameter, tee_ioctl_supp_recv() still goes
   to the same cleanup path without dropping already acquired shm refs.

This means the issue is not just a backend-specific quirk: the core ioctl
cleanup path itself is missing tee_shm_put() handling.

Impact:
- leaked shared-memory references
- shared-memory objects may remain pinned and never reach final release
- repeated triggering can cause resource exhaustion / DoS

This does not appear to be a UAF, since refcount_t saturation prevents
wraparound. The impact looks limited to resource leakage and availability.

If useful, I can also prepare a patch.

Thanks,
Qihang

Re: [REPORT] tee: tee_ioctl_supp_recv() missing tee_shm_put() cleanup for memref params

[Jens Wiklander]

Hi Qihang,

On Wed, Apr 29, 2026 at 5:07 AM Qihang <q.h.hack.winter@gmail.com> wrote:
>
> Hi,
>
> I would like to report a likely shared-memory reference leak in
> drivers/tee/tee_core.c, in tee_ioctl_supp_recv().
>
> The issue is that tee_ioctl_supp_recv() calls params_from_user(), which
> acquires references for MEMREF-type parameters via tee_shm_get_from_id(),
> but the cleanup path only frees the params array and does not drop the
> acquired shm references.
>
> Relevant code in tee_ioctl_supp_recv():
>
>   rc = params_from_user(ctx, params, num_params, uarg->params);
>   if (rc)
>           goto out;
>
>   rc = ctx->teedev->desc->ops->supp_recv(ctx, &func, &num_params, params);
>   if (rc)
>           goto out;
>   ...
> out:
>   kfree(params);
>   return rc;
>
> By contrast, other callers of params_from_user() in tee_core.c do release
> MEMREF references on cleanup, for example:
> - tee_ioctl_open_session()
> - tee_ioctl_invoke()
> - tee_ioctl_object_invoke()
>
> The helper comment in param_from_user_memref() also states that the caller
> is responsible for calling tee_shm_put() on all resolved pointers.
>
> Reachability / trigger paths:
>
> 1. qcomtee backend:
>    qcomtee registers .supp_recv = qcomtee_supp_recv on a non-privileged
>    tee device path. qcomtee_supp_recv() rejects any non-meta parameter
>    after the first one:
>      for (i = 1; i < *num_params; i++)
>              if (params[i].attr)
>                      return -EINVAL;
>    This allows a path where params_from_user() has already acquired a shm
>    reference for a MEMREF parameter, then qcomtee_supp_recv() returns an
>    error, and tee_ioctl_supp_recv() exits without tee_shm_put().
>
> 2. Partial failure before backend callback:
>    if params_from_user() successfully resolves an earlier MEMREF parameter
>    and then fails on a later parameter, tee_ioctl_supp_recv() still goes
>    to the same cleanup path without dropping already acquired shm refs.
>
> This means the issue is not just a backend-specific quirk: the core ioctl
> cleanup path itself is missing tee_shm_put() handling.
>
> Impact:
> - leaked shared-memory references
> - shared-memory objects may remain pinned and never reach final release
> - repeated triggering can cause resource exhaustion / DoS
>
> This does not appear to be a UAF, since refcount_t saturation prevents
> wraparound. The impact looks limited to resource leakage and availability.
>
> If useful, I can also prepare a patch.

Thanks for the detailed report. If possible, please prepare a patch to fix this.

Cheers,
Jens

>
> Thanks,
> Qihang

[PATCH] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Qihang]

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;
 }
 
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Sumit Garg via OP-TEE]

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

Re: [PATCH] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Qihang]

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

[PATCH v2] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Qihang]

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)

Re: [PATCH v2] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Qihang]

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

Re: [PATCH v2] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Jens Wiklander]

Hi Qihang,

On Wed, May 6, 2026 at 4:18 AM Qihang <q.h.hack.winter@gmail.com> wrote:
>
> 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?

qcomtee-specific changes should preferably go into a separate patch.
In this case, I suspect the fix is to add a similar tee_shm_put() as
we have in supp_check_recv_params() in drivers/tee/optee/supp.c.

Cheers,
Jens

Re: [PATCH v2] tee: fix missing shm reference cleanup in tee_ioctl_supp_recv

[Jens Wiklander]

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

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

[Qihang]

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

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

[Jens Wiklander]

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

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

[Qihang]

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>
---
v4:
- fold free_params() memref check onto one line
- remove unused local variables left behind by the cleanup refactoring

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 | 56 +++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index ef9642d72672..1aac50c7c1de 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -530,11 +530,24 @@ 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)
 {
 	int rc;
-	size_t n;
 	struct tee_ioctl_buf_data buf;
 	struct tee_ioctl_open_session_arg __user *uarg;
 	struct tee_ioctl_open_session_arg arg;
@@ -595,16 +608,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;
 }
 
@@ -612,7 +616,6 @@ static int tee_ioctl_invoke(struct tee_context *ctx,
 			    struct tee_ioctl_buf_data __user *ubuf)
 {
 	int rc;
-	size_t n;
 	struct tee_ioctl_buf_data buf;
 	struct tee_ioctl_invoke_arg __user *uarg;
 	struct tee_ioctl_invoke_arg arg;
@@ -657,14 +660,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;
 }
 
@@ -672,7 +668,6 @@ static int tee_ioctl_object_invoke(struct tee_context *ctx,
 				   struct tee_ioctl_buf_data __user *ubuf)
 {
 	int rc;
-	size_t n;
 	struct tee_ioctl_buf_data buf;
 	struct tee_ioctl_object_invoke_arg __user *uarg;
 	struct tee_ioctl_object_invoke_arg arg;
@@ -716,14 +711,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 +834,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)

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

[Jens Wiklander]

Hi,

On Thu, May 7, 2026 at 5:39 PM 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>
> ---
> v4:
> - fold free_params() memref check onto one line
> - remove unused local variables left behind by the cleanup refactoring
>
> 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 | 56 +++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 31 deletions(-)

Looks good. I'm picking up this.

Cheers,
Jens

>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index ef9642d72672..1aac50c7c1de 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -530,11 +530,24 @@ 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)
>  {
>         int rc;
> -       size_t n;
>         struct tee_ioctl_buf_data buf;
>         struct tee_ioctl_open_session_arg __user *uarg;
>         struct tee_ioctl_open_session_arg arg;
> @@ -595,16 +608,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;
>  }
>
> @@ -612,7 +616,6 @@ static int tee_ioctl_invoke(struct tee_context *ctx,
>                             struct tee_ioctl_buf_data __user *ubuf)
>  {
>         int rc;
> -       size_t n;
>         struct tee_ioctl_buf_data buf;
>         struct tee_ioctl_invoke_arg __user *uarg;
>         struct tee_ioctl_invoke_arg arg;
> @@ -657,14 +660,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;
>  }
>
> @@ -672,7 +668,6 @@ static int tee_ioctl_object_invoke(struct tee_context *ctx,
>                                    struct tee_ioctl_buf_data __user *ubuf)
>  {
>         int rc;
> -       size_t n;
>         struct tee_ioctl_buf_data buf;
>         struct tee_ioctl_object_invoke_arg __user *uarg;
>         struct tee_ioctl_object_invoke_arg arg;
> @@ -716,14 +711,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 +834,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)
>