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

[Jens Wiklander]

On Mon, Nov 9, 2026 at 7:27 AM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
>
> On Thu, 05 Nov 2026 at 13:56:51 +0100, Jens Wiklander wrote:
> >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?
> >
>
> Sounds reasonable to me, FWIW:
>
> Reviewed-by: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>

Good. I'm picking this up.

Cheers,
Jens