linux-kernel - RE: [PATCH v4 09/10] iommu: Make iommu_queue

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:   Wed, 13 Sep 2023 02:25:53 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        "Will Deacon" <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        "Jason Gunthorpe" <jgg@...pe.ca>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Nicolin Chen <nicolinc@...dia.com>
CC:     "Liu, Yi L" <yi.l.liu@...el.com>,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v4 09/10] iommu: Make iommu_queue_iopf() more generic

> From: Baolu Lu <baolu.lu@...ux.intel.com>
> Sent: Monday, September 11, 2023 8:27 PM
> 
> >
> > Out of curiosity. Is it a valid configuration which has
> REQUEST_PASID_VALID
> > set but RESP_PASID_VALID cleared? I'm unclear why another response
> > flag is required beyond what the request flag has told...
> 
> This seems to have uncovered a bug in VT-d driver.
> 
> The PCIe spec (Section 10.4.2.2) states:
> 
> "
> If a Page Request has a PASID, the corresponding PRG Response Message
> may optionally contain one as well.
> 
> If the PRG Response PASID Required bit is Clear, PRG Response Messages
> do not have a PASID. If the PRG Response PASID Required bit is Set, PRG
> Response Messages have a PASID if the Page Request also had one. The
> Function is permitted to use the PASID value from the prefix in
> conjunction with the PRG Index to match requests and responses.
> "
> 
> The "PRG Response PASID Required bit" is a read-only field in the PCI
> page request status register. It is represented by
> "pdev->pasid_required".
> 
> So below code in VT-d driver is not correct:
> 
> 542 static int intel_svm_prq_report(struct intel_iommu *iommu, struct
> device *dev,
> 543                                 struct page_req_dsc *desc)
> 544 {
> 
> [...]
> 
> 556
> 557         if (desc->lpig)
> 558                 event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE;
> 559         if (desc->pasid_present) {
> 560                 event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> 561                 event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
> 562         }
> [...]
> 
> The right logic should be
> 
> 	if (pdev->pasid_required)
> 		event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
> 
> Thoughts?
> 

yes, it's the right fix. We haven't seen any bug report probably because
all SVM-capable devices have pasid_required set? 😊