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

[Sumit Garg]

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>

-Sumit