Re: [PATCH V1] iommu/sva: Fix crash in iommu_sva_unbind_device()

[Lizhi Hou]

On 2/25/26 01:35, Yi Liu wrote:
> [You don't often get email from yi.l.liu@xxxxxxxxx. Learn why this is 
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On 2026/2/25 02:30, Lizhi Hou wrote:
> > domain->mm->iommu_mm can be freed by iommu_domain_free():
> >    iommu_domain_free()
> >      mmdrop()
> >        __mmdrop()
> >          mm_pasid_drop()
> > After iommu_domain_free() returns, accessing domain->mm->iommu_mm may
> > dereference a freed mm structure, leading to a crash.
> >
> > Fix this by taking a reference to the mm via mmgrab() before
> > calling iommu_domain_free(), and dropping it with mmdrop() after
> > finishing access to domain->mm->iommu_mm.
>
> need to be more accurate. The issue is that iommu_mm is freed in
> mm_pasid_drop(), so after iommu_domain_free(), the later access to
> iommu_mm is problematic.

The description mentioned this.

After iommu_domain_free() returns, accessing domain->mm->iommu_mm may
dereference a freed mm structure, leading to a crash.

> > Fixes: e37d5a2d60a3 ("iommu/sva: invalidate stale IOTLB entries for 
> > kernel address space")
> > Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
> > ---
> >   drivers/iommu/iommu-sva.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> > index 07d64908a05f..523b8c65c86f 100644
> > --- a/drivers/iommu/iommu-sva.c
> > +++ b/drivers/iommu/iommu-sva.c
> > @@ -179,6 +179,7 @@ void iommu_sva_unbind_device(struct iommu_sva 
> > *handle)
> >               return;
> >       }
> >
> > +     mmgrab(domain->mm);
> >       iommu_detach_device_pasid(domain, dev, iommu_mm->pasid);
> >       if (--domain->users == 0) {
> >               list_del(&domain->next);
> > @@ -190,6 +191,7 @@ void iommu_sva_unbind_device(struct iommu_sva 
> > *handle)
> >               if (list_empty(&iommu_sva_mms))
> >                       iommu_sva_present = false;
> >       }
> > +     mmdrop(domain->mm);
> >
> >       mutex_unlock(&iommu_sva_lock);
> >       kfree(handle);
>
> will moving the below hunk in front of iommu_domain_free() simpler?
> Only when (--domain->users == 0), shall the code check if sva_domains
> is empty. right?

I am not sure if this can be moved in front of iommu_domain_free(). Will 
iommu_domain_free() be possible to impact sva_domains?

iommu_domain_free() calls domain->ops->free(). Could this call back free 
sva_domain?


Lizhi

>         if (list_empty(&iommu_mm->sva_domains)) {
>             list_del(&iommu_mm->mm_list_elm);
>             if (list_empty(&iommu_sva_mms))
>                 iommu_sva_present = false;
>         }
>
> Regards,
> Yi Liu