linux-kernel - RE: [PATCH v4 04/12] iommu/vt-d: Add 256-bit invalidation descriptor support

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]

Message-ID: <A2975661238FB949B60364EF0F2C257439D6061D@SHSMSX104.ccr.corp.intel.com>
Date:   Thu, 8 Nov 2018 07:20:44 +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>,
        "Kumar, Sanjay K" <sanjay.k.kumar@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        "Sun, Yi Y" <yi.y.sun@...el.com>,
        "peterx@...hat.com" <peterx@...hat.com>,
        Jean-Philippe Brucker <jean-philippe.brucker@....com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: RE: [PATCH v4 04/12] iommu/vt-d: Add 256-bit invalidation
 descriptor support

Hi,

> From: Lu Baolu [mailto:baolu.lu@...ux.intel.com]
> Sent: Thursday, November 8, 2018 2:14 PM
> 
> Hi,
> 
> On 11/8/18 1:45 PM, Liu, Yi L wrote:
> >> From: Lu Baolu [mailto:baolu.lu@...ux.intel.com]
> >> Sent: Thursday, November 8, 2018 1:25 PM
> >> Subject: Re: [PATCH v4 04/12] iommu/vt-d: Add 256-bit invalidation
> >> descriptor support
> >>
> >> Hi,
> >>
> >> On 11/8/18 11:49 AM, Liu, Yi L wrote:
> >>> Hi,
> >>>
> >>>> From: Lu Baolu [mailto:baolu.lu@...ux.intel.com]
> >>>> Sent: Thursday, November 8, 2018 10:17 AM
> >>>> Subject: Re: [PATCH v4 04/12] iommu/vt-d: Add 256-bit invalidation
> >>>> descriptor support
> >>>>
> >>>> Hi Yi,
> >>>>
> >>>> On 11/7/18 2:07 PM, Liu, Yi L wrote:
> >>>>> Hi Baolu,
> >>>>>
> >>>>>> From: Lu Baolu [mailto:baolu.lu@...ux.intel.com]
> >>>>>> Sent: Monday, November 5, 2018 1:32 PM
> >>>>> [...]
> >>>>>
> >>>>>> ---
> >>>>>>     drivers/iommu/dmar.c                | 83 +++++++++++++++++++----------
> >>>>>>     drivers/iommu/intel-svm.c           | 76 ++++++++++++++++----------
> >>>>>>     drivers/iommu/intel_irq_remapping.c |  6 ++-
> >>>>>>     include/linux/intel-iommu.h         |  9 +++-
> >>>>>>     4 files changed, 115 insertions(+), 59 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c index
> >>>>>> d9c748b6f9e4..ec10427b98ac 100644
> >>>>>> --- a/drivers/iommu/dmar.c
> >>>>>> +++ b/drivers/iommu/dmar.c
> >>>>>> @@ -1160,6 +1160,7 @@ static int qi_check_fault(struct
> >>>>>> intel_iommu *iommu, int
> >>>>>> index)
> >>>>>>     	int head, tail;
> >>>>>>     	struct q_inval *qi = iommu->qi;
> >>>>>>     	int wait_index = (index + 1) % QI_LENGTH;
> >>>>>> +	int shift = qi_shift(iommu);
> >>>>>>
> >>>>>>     	if (qi->desc_status[wait_index] == QI_ABORT)
> >>>>>>     		return -EAGAIN;
> >>>>>> @@ -1173,13 +1174,15 @@ static int qi_check_fault(struct
> >>>>>> intel_iommu *iommu, int index)
> >>>>>>     	 */
> >>>>>>     	if (fault & DMA_FSTS_IQE) {
> >>>>>>     		head = readl(iommu->reg + DMAR_IQH_REG);
> >>>>>> -		if ((head >> DMAR_IQ_SHIFT) == index) {
> >>>>>> +		if ((head >> shift) == index) {
> >>>>>> +			struct qi_desc *desc = qi->desc + head;
> >>>>>> +
> >>>>>>     			pr_err("VT-d detected invalid descriptor: "
> >>>>>>     				"low=%llx, high=%llx\n",
> >>>>>> -				(unsigned long long)qi->desc[index].low,
> >>>>>> -				(unsigned long long)qi->desc[index].high);
> >>>>>> -			memcpy(&qi->desc[index], &qi->desc[wait_index],
> >>>>>> -					sizeof(struct qi_desc));
> >>>>>> +				(unsigned long long)desc->qw0,
> >>>>>> +				(unsigned long long)desc->qw1);
> >>>>> Still missing qw2 and qw3. May make the print differ based on if smts is
> configed.
> >>>> qw2 and qw3 are reserved from software point of view. We don't need
> >>>> to print it for information.
> >>> But for Scalable mode, it should be valid?
> >> No. It's reserved for software.
> > No, I don’t think so. PRQ response would also be queued to hardware by
> > QI. For such QI descriptors, the high bits are not reserved.
> >
> 
> Do you mean the private data fields of a page request descriptor or a page group
> response descriptor? Those fields contains software defined private data (might a
> kernel pointer?). We should avoid leaking such information in the generic kernel
> message for security consideration.
> Or anything I missed?

yes, I'm not sure what kind of data it may be in the private data field. From software
point of view, it may be helpful to show the full content of the QI descriptor for error
triage. Personally, I'm fine if you keep it on this point.

Regards,
Yi Liu