* [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() @ 2018-11-05 2:18 Lu Baolu 2018-11-05 5:21 ` Liu, Yi L 2018-11-06 15:48 ` Joerg Roedel 0 siblings, 2 replies; 5+ messages in thread From: Lu Baolu @ 2018-11-05 2:18 UTC (permalink / raw) To: Joerg Roedel, David Woodhouse Cc: ashok.raj, iommu, linux-kernel, Lu Baolu, Jacob Pan, Sohil Mehta When handling page request without pasid event, go to "no_pasid" branch instead of "bad_req". Otherwise, a NULL pointer deference will happen there. Cc: Ashok Raj <ashok.raj@intel.com> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> Cc: Sohil Mehta <sohil.mehta@intel.com> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> --- drivers/iommu/intel-svm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c index db301efe126d..887150907526 100644 --- a/drivers/iommu/intel-svm.c +++ b/drivers/iommu/intel-svm.c @@ -595,7 +595,7 @@ static irqreturn_t prq_event_thread(int irq, void *d) pr_err("%s: Page request without PASID: %08llx %08llx\n", iommu->name, ((unsigned long long *)req)[0], ((unsigned long long *)req)[1]); - goto bad_req; + goto no_pasid; } if (!svm || svm->pasid != req->pasid) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() 2018-11-05 2:18 [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() Lu Baolu @ 2018-11-05 5:21 ` Liu, Yi L 2018-11-05 5:44 ` Lu Baolu 2018-11-06 15:48 ` Joerg Roedel 1 sibling, 1 reply; 5+ messages in thread From: Liu, Yi L @ 2018-11-05 5:21 UTC (permalink / raw) To: Lu Baolu, Joerg Roedel, David Woodhouse; +Cc: Raj, Ashok, linux-kernel, iommu Hi Baolu, > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu- > bounces@lists.linux-foundation.org] On Behalf Of Lu Baolu > Sent: Monday, November 5, 2018 10:19 AM > To: Joerg Roedel <joro@8bytes.org>; David Woodhouse <dwmw2@infradead.org> > Cc: Raj, Ashok <ashok.raj@intel.com>; linux-kernel@vger.kernel.org; > iommu@lists.linux-foundation.org > Subject: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() > > When handling page request without pasid event, go to "no_pasid" > branch instead of "bad_req". Otherwise, a NULL pointer deference will happen there. > > Cc: Ashok Raj <ashok.raj@intel.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Sohil Mehta <sohil.mehta@intel.com> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > drivers/iommu/intel-svm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c index > db301efe126d..887150907526 100644 > --- a/drivers/iommu/intel-svm.c > +++ b/drivers/iommu/intel-svm.c > @@ -595,7 +595,7 @@ static irqreturn_t prq_event_thread(int irq, void *d) > pr_err("%s: Page request without PASID: %08llx %08llx\n", > iommu->name, ((unsigned long long *)req)[0], > ((unsigned long long *)req)[1]); > - goto bad_req; > + goto no_pasid; > } > > if (!svm || svm->pasid != req->pasid) { > -- I'm afraid it is still necessary to goto "bad_req". The following code behind "bad_req" will trigger fault_cb registered by in-kernel drivers. It is reasonable that PRQ without PASID can be handled by such callbacks. So I would suggest to keep the existing logic. if (sdev && sdev->ops && sdev->ops->fault_cb) { int rwxp = (req->rd_req << 3) | (req->wr_req << 2) | (req->exe_req << 1) | (req->priv_req); sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, req->private, rwxp, result); } Thanks, Yi Liu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() 2018-11-05 5:21 ` Liu, Yi L @ 2018-11-05 5:44 ` Lu Baolu 2018-11-05 7:08 ` Liu, Yi L 0 siblings, 1 reply; 5+ messages in thread From: Lu Baolu @ 2018-11-05 5:44 UTC (permalink / raw) To: Liu, Yi L, Joerg Roedel, David Woodhouse Cc: baolu.lu, Raj, Ashok, linux-kernel, iommu Hi Yi, On 11/5/18 1:21 PM, Liu, Yi L wrote: > Hi Baolu, > >> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu- >> bounces@lists.linux-foundation.org] On Behalf Of Lu Baolu >> Sent: Monday, November 5, 2018 10:19 AM >> To: Joerg Roedel <joro@8bytes.org>; David Woodhouse <dwmw2@infradead.org> >> Cc: Raj, Ashok <ashok.raj@intel.com>; linux-kernel@vger.kernel.org; >> iommu@lists.linux-foundation.org >> Subject: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() >> >> When handling page request without pasid event, go to "no_pasid" >> branch instead of "bad_req". Otherwise, a NULL pointer deference will happen there. >> >> Cc: Ashok Raj <ashok.raj@intel.com> >> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> >> Cc: Sohil Mehta <sohil.mehta@intel.com> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> --- >> drivers/iommu/intel-svm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c index >> db301efe126d..887150907526 100644 >> --- a/drivers/iommu/intel-svm.c >> +++ b/drivers/iommu/intel-svm.c >> @@ -595,7 +595,7 @@ static irqreturn_t prq_event_thread(int irq, void *d) >> pr_err("%s: Page request without PASID: %08llx %08llx\n", >> iommu->name, ((unsigned long long *)req)[0], >> ((unsigned long long *)req)[1]); >> - goto bad_req; >> + goto no_pasid; >> } >> >> if (!svm || svm->pasid != req->pasid) { >> -- > > I'm afraid it is still necessary to goto "bad_req". The following code behind > "bad_req" will trigger fault_cb registered by in-kernel drivers. It is reasonable > that PRQ without PASID can be handled by such callbacks. So I would suggest > to keep the existing logic. A page fault without a pasid is triggered by a DMA transfer without PASID. It doesn't relate to the SVM functionality hence there's no @svm or @sdev related to it. It's unnecessary to report it to the drivers as far as I can see. Best regards, Lu Baolu > > if (sdev && sdev->ops && sdev->ops->fault_cb) { > int rwxp = (req->rd_req << 3) | (req->wr_req << 2) | > (req->exe_req << 1) | (req->priv_req); > sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, req->private, rwxp, result); > } > > Thanks, > Yi Liu > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() 2018-11-05 5:44 ` Lu Baolu @ 2018-11-05 7:08 ` Liu, Yi L 0 siblings, 0 replies; 5+ messages in thread From: Liu, Yi L @ 2018-11-05 7:08 UTC (permalink / raw) To: Lu Baolu, Joerg Roedel, David Woodhouse Cc: Raj, Ashok, linux-kernel, iommu, Pan, Jacob jun > From: Lu Baolu [mailto:baolu.lu@linux.intel.com] > Sent: Monday, November 5, 2018 1:45 PM > To: Liu, Yi L <yi.l.liu@intel.com>; Joerg Roedel <joro@8bytes.org>; David > Woodhouse <dwmw2@infradead.org> > Cc: baolu.lu@linux.intel.com; Raj, Ashok <ashok.raj@intel.com>; linux- > kernel@vger.kernel.org; iommu@lists.linux-foundation.org > Subject: Re: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in > prq_event_thread() > > Hi Yi, > > On 11/5/18 1:21 PM, Liu, Yi L wrote: > > Hi Baolu, > > > >> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu- > >> bounces@lists.linux-foundation.org] On Behalf Of Lu Baolu > >> Sent: Monday, November 5, 2018 10:19 AM > >> To: Joerg Roedel <joro@8bytes.org>; David Woodhouse > >> <dwmw2@infradead.org> > >> Cc: Raj, Ashok <ashok.raj@intel.com>; linux-kernel@vger.kernel.org; > >> iommu@lists.linux-foundation.org > >> Subject: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in > >> prq_event_thread() > >> > >> When handling page request without pasid event, go to "no_pasid" > >> branch instead of "bad_req". Otherwise, a NULL pointer deference will happen > there. > >> > >> Cc: Ashok Raj <ashok.raj@intel.com> > >> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > >> Cc: Sohil Mehta <sohil.mehta@intel.com> > >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > >> --- > >> drivers/iommu/intel-svm.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c > >> index > >> db301efe126d..887150907526 100644 > >> --- a/drivers/iommu/intel-svm.c > >> +++ b/drivers/iommu/intel-svm.c > >> @@ -595,7 +595,7 @@ static irqreturn_t prq_event_thread(int irq, void *d) > >> pr_err("%s: Page request without PASID: %08llx %08llx\n", > >> iommu->name, ((unsigned long long *)req)[0], > >> ((unsigned long long *)req)[1]); > >> - goto bad_req; > >> + goto no_pasid; > >> } > >> > >> if (!svm || svm->pasid != req->pasid) { > >> -- > > > > I'm afraid it is still necessary to goto "bad_req". The following code > > behind "bad_req" will trigger fault_cb registered by in-kernel > > drivers. It is reasonable that PRQ without PASID can be handled by > > such callbacks. So I would suggest to keep the existing logic. > > A page fault without a pasid is triggered by a DMA transfer without PASID. It doesn't > relate to the SVM functionality hence there's no @svm or @sdev related to it. It's > unnecessary to report it to the drivers as far as I can see. Yeah, a PRQ without PASID has no corresponding svm or sdev structure. Regards to this fact, it's acceptable for this fix. In long term, it might be helpful to refine the PRQ event handler to cover PRQ without PASID. I guess Jacob's fault report framework may help. +Jacob Regards, Yi Liu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() 2018-11-05 2:18 [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() Lu Baolu 2018-11-05 5:21 ` Liu, Yi L @ 2018-11-06 15:48 ` Joerg Roedel 1 sibling, 0 replies; 5+ messages in thread From: Joerg Roedel @ 2018-11-06 15:48 UTC (permalink / raw) To: Lu Baolu Cc: David Woodhouse, ashok.raj, iommu, linux-kernel, Jacob Pan, Sohil Mehta On Mon, Nov 05, 2018 at 10:18:58AM +0800, Lu Baolu wrote: > When handling page request without pasid event, go to "no_pasid" > branch instead of "bad_req". Otherwise, a NULL pointer deference > will happen there. > > Cc: Ashok Raj <ashok.raj@intel.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Sohil Mehta <sohil.mehta@intel.com> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > drivers/iommu/intel-svm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied for v4.20, thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-06 15:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-05 2:18 [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event_thread() Lu Baolu 2018-11-05 5:21 ` Liu, Yi L 2018-11-05 5:44 ` Lu Baolu 2018-11-05 7:08 ` Liu, Yi L 2018-11-06 15:48 ` Joerg Roedel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).