From: Zhangfei Gao <zhangfei.gao@linaro.org> To: Fenghua Yu <fenghua.yu@intel.com>, Dave Hansen <dave.hansen@linux.intel.com>, Thomas Gleixner <tglx@linutronix.de>, Jean-Philippe Brucker <jean-philippe@linaro.org>, Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>, Zhangfei Gao <zhangfei.gao@linaro.org>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, Tony Luck <tony.luck@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>, Ravi V Shankar <ravi.v.shankar@intel.com>, Peter Zijlstra <peterz@infradead.org>, Andy Lutomirski <luto@kernel.org>, x86 <x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>, iommu@lists.linux-foundation.org Subject: Re: [PATCH v2] iommu/sva: Fix PASID use-after-free issue Date: Fri, 29 Apr 2022 09:39:09 +0800 [thread overview] Message-ID: <8f50c673-fe92-3c42-993d-43e65fc7235c@linaro.org> (raw) In-Reply-To: <20220428180041.806809-1-fenghua.yu@intel.com> On 2022/4/29 上午2:00, Fenghua Yu wrote: > The PASID is being freed too early. It needs to stay around until after > device drivers that might be using it have had a chance to clear it out > of the hardware. > > As a reminder: > > mmget() /mmput() refcount the mm's address space > mmgrab()/mmdrop() refcount the mm itself > > The PASID is currently tied to the life of the mm's address space and > freed in __mmput(). This makes logical sense because the PASID can't be > used once the address space is gone. > > But, this misses an important point: even after the address space is > gone, the PASID will still be programmed into a device. Device drivers > might, for instance, still need to flush operations that are outstanding > and need to use that PASID. They do this at file->release() time. > > Device drivers call the IOMMU driver to hold a reference on the mm itself > and drop it at file->release() time. But, the IOMMU driver holds a > reference on the mm itself, not the address space. The address space > (and the PASID) is long gone by the time the driver tries to clean up. > This is effectively a use-after-free bug on the PASID. > > To fix this, move the PASID free operation from __mmput() to __mmdrop(). > This ensures that the IOMMU driver's existing mmgrab() keeps the PASID > allocated until it drops its mm reference. > > Fixes: 701fac40384f ("iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit") > > Reported-by: Zhangfei Gao <zhangfei.gao@foxmail.com> > Tested-by: Zhangfei Gao <zhangfei.gao@foxmail.com> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org> Use the formal email, thanks > Suggested-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > --- > > v2: > - Dave Hansen rewrites the change log. > - Add Tested-by: Zhangfei Gao <zhangfei.gao@foxmail.com> > - Add Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > The original patch was posted and discussed in: > https://lore.kernel.org/lkml/YmdzFFx7fN586jcf@fyu1.sc.intel.com/ > > kernel/fork.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 9796897560ab..35a3beff140b 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -792,6 +792,7 @@ void __mmdrop(struct mm_struct *mm) > mmu_notifier_subscriptions_destroy(mm); > check_mm(mm); > put_user_ns(mm->user_ns); > + mm_pasid_drop(mm); > free_mm(mm); > } > EXPORT_SYMBOL_GPL(__mmdrop); > @@ -1190,7 +1191,6 @@ static inline void __mmput(struct mm_struct *mm) > } > if (mm->binfmt) > module_put(mm->binfmt->module); > - mm_pasid_drop(mm); > mmdrop(mm); > } >
WARNING: multiple messages have this Message-ID (diff)
From: Zhangfei Gao <zhangfei.gao@linaro.org> To: Fenghua Yu <fenghua.yu@intel.com>, Dave Hansen <dave.hansen@linux.intel.com>, Thomas Gleixner <tglx@linutronix.de>, Jean-Philippe Brucker <jean-philippe@linaro.org>, Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>, Zhangfei Gao <zhangfei.gao@linaro.org>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, Tony Luck <tony.luck@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>, Ravi V Shankar <ravi.v.shankar@intel.com>, Peter Zijlstra <peterz@infradead.org>, Andy Lutomirski <luto@kernel.org>, x86 <x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>, iommu@lists.linux-foundation.org Subject: Re: [PATCH v2] iommu/sva: Fix PASID use-after-free issue Date: Fri, 29 Apr 2022 09:39:09 +0800 [thread overview] Message-ID: <8f50c673-fe92-3c42-993d-43e65fc7235c@linaro.org> (raw) In-Reply-To: <20220428180041.806809-1-fenghua.yu@intel.com> On 2022/4/29 上午2:00, Fenghua Yu wrote: > The PASID is being freed too early. It needs to stay around until after > device drivers that might be using it have had a chance to clear it out > of the hardware. > > As a reminder: > > mmget() /mmput() refcount the mm's address space > mmgrab()/mmdrop() refcount the mm itself > > The PASID is currently tied to the life of the mm's address space and > freed in __mmput(). This makes logical sense because the PASID can't be > used once the address space is gone. > > But, this misses an important point: even after the address space is > gone, the PASID will still be programmed into a device. Device drivers > might, for instance, still need to flush operations that are outstanding > and need to use that PASID. They do this at file->release() time. > > Device drivers call the IOMMU driver to hold a reference on the mm itself > and drop it at file->release() time. But, the IOMMU driver holds a > reference on the mm itself, not the address space. The address space > (and the PASID) is long gone by the time the driver tries to clean up. > This is effectively a use-after-free bug on the PASID. > > To fix this, move the PASID free operation from __mmput() to __mmdrop(). > This ensures that the IOMMU driver's existing mmgrab() keeps the PASID > allocated until it drops its mm reference. > > Fixes: 701fac40384f ("iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit") > > Reported-by: Zhangfei Gao <zhangfei.gao@foxmail.com> > Tested-by: Zhangfei Gao <zhangfei.gao@foxmail.com> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org> Use the formal email, thanks > Suggested-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Suggested-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > --- > > v2: > - Dave Hansen rewrites the change log. > - Add Tested-by: Zhangfei Gao <zhangfei.gao@foxmail.com> > - Add Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > The original patch was posted and discussed in: > https://lore.kernel.org/lkml/YmdzFFx7fN586jcf@fyu1.sc.intel.com/ > > kernel/fork.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 9796897560ab..35a3beff140b 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -792,6 +792,7 @@ void __mmdrop(struct mm_struct *mm) > mmu_notifier_subscriptions_destroy(mm); > check_mm(mm); > put_user_ns(mm->user_ns); > + mm_pasid_drop(mm); > free_mm(mm); > } > EXPORT_SYMBOL_GPL(__mmdrop); > @@ -1190,7 +1191,6 @@ static inline void __mmput(struct mm_struct *mm) > } > if (mm->binfmt) > module_put(mm->binfmt->module); > - mm_pasid_drop(mm); > mmdrop(mm); > } > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2022-04-29 1:39 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-28 18:00 [PATCH v2] iommu/sva: Fix PASID use-after-free issue Fenghua Yu 2022-04-28 18:00 ` Fenghua Yu 2022-04-29 1:39 ` Zhangfei Gao [this message] 2022-04-29 1:39 ` Zhangfei Gao 2022-05-06 1:49 ` Zhangfei Gao 2022-05-06 1:49 ` Zhangfei Gao 2022-05-06 7:27 ` Thomas Gleixner 2022-05-06 7:27 ` Thomas Gleixner 2022-05-01 8:25 ` [tip: core/urgent] mm: " tip-bot2 for Fenghua Yu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=8f50c673-fe92-3c42-993d-43e65fc7235c@linaro.org \ --to=zhangfei.gao@linaro.org \ --cc=bp@alien8.de \ --cc=dave.hansen@linux.intel.com \ --cc=fenghua.yu@intel.com \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@linux.intel.com \ --cc=jean-philippe@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@kernel.org \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=ravi.v.shankar@intel.com \ --cc=robin.murphy@arm.com \ --cc=tglx@linutronix.de \ --cc=tony.luck@intel.com \ --cc=will@kernel.org \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.