Re: [PATCH] tee: avoid accessing dma-buf shared memory after put

[Jens Wiklander]

Hi,

On Thu, Aug 6, 2026 at 8:07 AM Hongyan Xu <getshell@xxxxxxxxxx> wrote:
>
> param_from_user_memref() obtains a temporary reference to the dma-buf
> tee_shm wrapper by ID. If closing the shared-memory file descriptor races
> with the ioctl, this temporary reference can be the wrapper's last one.
> tee_shm_put() then frees the enclosing tee_shm_dmabuf_ref, but the function
> still reads ref->parent_shm and ref->offset afterwards.
>
> Save the parent pointer and offset before dropping the wrapper reference.
> The additional parent reference keeps the parent shared memory alive for
> the parameter list as before.
>
> Fixes: 146bf4e75eca ("tee: new ioctl to a register tee_shm from a dmabuf file descriptor")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hongyan Xu <getshell@xxxxxxxxxx>
> ---
>  drivers/tee/tee_core.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 1aac50c7c1de..489c39d5b35f 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -397,6 +397,7 @@ static int param_from_user_memref(struct tee_context *ctx,
>                 }
>
>                 if (shm->flags & TEE_SHM_DMA_BUF) {
> +                       struct tee_shm *parent_shm;
>                         struct tee_shm_dmabuf_ref *ref;
>
>                         ref = container_of(shm, struct tee_shm_dmabuf_ref, shm);
> @@ -409,10 +410,11 @@ static int param_from_user_memref(struct tee_context *ctx,
>                                  * list instead of the shm we got with
>                                  * tee_shm_get_from_id() above.
>                                  */
> -                               refcount_inc(&ref->parent_shm->refcount);
> -                               tee_shm_put(shm);
> -                               shm = ref->parent_shm;
> +                               parent_shm = ref->parent_shm;
>                                 offs = ref->offset;
> +                               refcount_inc(&parent_shm->refcount);
> +                               tee_shm_put(shm);
> +                               shm = parent_shm;
>                         }
>                 }
>         } else if (ctx->cap_memref_null) {
> --
> 2.50.1.windows.1
>

Looks good. Sumit, do you agree?

Cheers,
Jens