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

[Sumit Garg <sumit.garg@kernel.org>]

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@seu.edu.cn> 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@vger.kernel.org
>> Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
>> ---
>>  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@oss.qualcomm.com>

-Sumit