linux-kernel - RE: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in prq_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 5 Nov 2018 07:08:52 +0000
From:   "Liu, Yi L" <yi.l.liu@...el.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        "David Woodhouse" <dwmw2@...radead.org>
CC:     "Raj, Ashok" <ashok.raj@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>
Subject: RE: [PATCH 1/1] iommu/vtd: Fix NULL pointer dereference in
 prq_event_thread()

> From: Lu Baolu [mailto:baolu.lu@...ux.intel.com]
> Sent: Monday, November 5, 2018 1:45 PM
> To: Liu, Yi L <yi.l.liu@...el.com>; Joerg Roedel <joro@...tes.org>; David
> Woodhouse <dwmw2@...radead.org>
> Cc: baolu.lu@...ux.intel.com; Raj, Ashok <ashok.raj@...el.com>; linux-
> kernel@...r.kernel.org; iommu@...ts.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@...ts.linux-foundation.org [mailto:iommu-
> >> bounces@...ts.linux-foundation.org] On Behalf Of Lu Baolu
> >> Sent: Monday, November 5, 2018 10:19 AM
> >> To: Joerg Roedel <joro@...tes.org>; David Woodhouse
> >> <dwmw2@...radead.org>
> >> Cc: Raj, Ashok <ashok.raj@...el.com>; linux-kernel@...r.kernel.org;
> >> iommu@...ts.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@...el.com>
> >> Cc: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> >> Cc: Sohil Mehta <sohil.mehta@...el.com>
> >> Signed-off-by: Lu Baolu <baolu.lu@...ux.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