linux-kernel - RE: [PATCH] thunderbolt: Stop using iommu

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, 16 Mar 2022 17:53:42 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@....com>
To:     Robin Murphy <robin.murphy@....com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>
CC:     "michael.jamet@...el.com" <michael.jamet@...el.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "YehezkelShB@...il.com" <YehezkelShB@...il.com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "andreas.noever@...il.com" <andreas.noever@...il.com>,
        "hch@....de" <hch@....de>
Subject: RE: [PATCH] thunderbolt: Stop using iommu_present()

[Public]

> >>>
> >>> There is a way to figure out the "tunneled" PCIe ports by looking at
> >>> certain properties and we do that already actually. The BIOS has the
> >>> following under these ports:
> >>>
> >>>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs
> >>> .microsoft.com%2Fen-us%2Fwindows-
> hardware%2Fdrivers%2Fpci%2Fdsd-
> >>> for-pcie-root-ports%23identifying-externally-exposed-pcie-root-
> >>>
> ports&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C0465d319a
> >>>
> 6684335d9c208da07710e7c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> >>>
> C0%7C637830479402895833%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> >>>
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
> >>>
> p;sdata=z6hpYGpj%2B%2BVvz9d6MXiO4N66PUm4zwhOdI%2Br6l3PjhQ%3D
> >>> &amp;reserved=0
> >>>
> >>> and the ports will have dev->external_facing set to 1. Perhaps looking
> >>> at that field helps here?
> >>
> >> External facing isn't a guarantee from the firmware though.  It's
> something we
> >> all expect in practice, but I think it's better to look at the ones that are
> from
> >> the _DSD usb4-host-interface to be safer.
> >
> > Right but then we have the discrete ones with the DVSEC that exposes the
> > tunneled ports :(
> >

Can the USB4 CM make the device links in the DVSEC case perhaps too?  I would
think we want that anyway to control device suspend ordering.

If I had something discrete to try I'd dust off the DVSEC patch I wrote before to
try it, but alas all I have is integrated stuff on my hand.

> >> Mika, you might not have seen it yet, but I sent a follow up diff in this
> thread
> >> to Robin's patch.  If that looks good Robin can submit a v2 (or I'm happy to
> do
> >> so as well as I confirmed it helps my original intent too).
> >
> > I saw it now and I'm thinking are we making this unnecessary complex? I
> > mean Microsoft solely depends on the DMAR platform opt-in flag:
> >
> >
> 

I think Microsoft doesn't allow you to turn off the IOMMU though or put it in
passthrough through on the kernel command line.

> > We also do turn on full IOMMU mappings in that case for devices that are
> > marked as external facing by the same firmware that provided the DMAR
> > bit. If the user decides to disable IOMMU from command line for instance
> > then we expect she knows what she is doing.
> 
> Yeah, if external_facing is set correctly then we can safely expect the
> the IOMMU layer to do the right thing, so in that case it probably is OK
> to infer that if an IOMMU is present for the NHI then it'll be managing
> that whole bus hierarchy. What I'm really thinking about here is whether
> we can defend against a case when external_facing *isn't* set, so we
> treat the tunnelled ports as normal PCI buses, assume it's OK since
> we've got an IOMMU and everything else is getting translation domains by
> default, but then a Thunderbolt device shows up masquerading the VID:DID
> of something that gets a passthrough quirk, and thus tricks its way
> through the perceived protection.
> 
> Robin.

Unless it happened after 5.17-rc8 looking at the code I think that's Intel
specific behavior though at the moment (has_external_pci).  I don't see it
in a generic layer.

In addition to the point Robin said about firmware not setting external facing
if the IOMMU was disabled on command line then iommu_dma_protection
would be showing the wrong values meaning userspace may choose to
authorize the device automatically in a potentially unsafe scenario.

Even if the user "knew what they were doing", I would expect that we still
do our best to protect them from themselves and not advertise something
that will cause automatic authorization.