linux-kernel - RE: [PATCH RFC v2 02/11] iommu: Add iommu_group_singleton

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, 30 Mar 2022 06:50:11 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Jason Gunthorpe <jgg@...dia.com>
CC:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        Christoph Hellwig <hch@...radead.org>,
        "Raj, Ashok" <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        Eric Auger <eric.auger@...hat.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH RFC v2 02/11] iommu: Add iommu_group_singleton_lockdown()

> From: Jason Gunthorpe <jgg@...dia.com>
> Sent: Tuesday, March 29, 2022 7:43 PM
> 
> On Tue, Mar 29, 2022 at 08:42:13AM +0000, Tian, Kevin wrote:
> 
> > btw I'm not sure whether this is what SVA requires. IIRC the problem with
> > SVA is because PASID TLP prefix is not counted in PCI packet routing thus
> > a DMA target address with PASID might be treated as P2P if the address
> > falls into the MMIO BAR of other devices in the group. This is why the
> > original code needs to strictly apply SVA in a group containing a single
> > device, instead of a group attached by a single driver, unless we want to
> > reserve those MMIO ranges in CPU VA space.
> 
> I think it is not such a good idea to mix up group with this test
> 
> Here you want to say that all TLPs from the RID route to the host
> bridge - ie ACS is on/etc. This is subtly different from a group with
> a single device. Specifically it is an immutable property of the
> fabric and doesn't change after hot plug events.

Yes, in concept your description is more accurate.

However according to pci_device_group() a group is created mainly
for lacking of ACS. Without ACS there is no guarantee that all TLPs
from the RID in a multi-devices group are routed to the host bridge.
In this case only singleton group can meet this requirement.

One thing that I'm not very sure is about DMA alias. Even when physically
there is only a single device within the group the aliasing could lead
to multiple RIDs in the group making it non-singleton. But probably we
don't need support SVA on such device until a real demand comes?

> 
> ie if we have a singleton group that doesn't have ACS and someone
> hotplugs in another device on a bridge, then our SVA is completely
> broken and we get data corruption.

Can we capture that in iommu_probe_device() when identifying
the group which the probed device will be added to has already been
locked down for SVA? i.e. make iommu_group_singleton_lockdown()
in this patch to lock down the fact of singleton group instead of
the fact of singleton driver...

> 
> Testing the group size is inherently the wrong test to make.
> 

What is your suggestion then?

Thanks
Kevin