Hi, On Thu, Aug 22, 2024 at 10:29:55AM -0500, Mario Limonciello wrote: > On 8/15/2024 14:07, Esther Shimanovich wrote: > > Some computers with CPUs that lack Thunderbolt features use discrete > > Thunderbolt chips to add Thunderbolt functionality. These Thunderbolt > > chips are located within the chassis; between the root port labeled > > ExternalFacingPort and the USB-C port. > > > > These Thunderbolt PCIe devices should be labeled as fixed and trusted, > > as they are built into the computer. Otherwise, security policies that > > rely on those flags may have unintended results, such as preventing > > USB-C ports from enumerating. > > > > Detect the above scenario through the process of elimination. > > > > 1) Integrated Thunderbolt host controllers already have Thunderbolt > > implemented, so anything outside their external facing root port is > > removable and untrusted. > > > > Detect them using the following properties: > > > > - Most integrated host controllers have the usb4-host-interface > > ACPI property, as described here: > > Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#mapping-native-protocols-pcie-displayport-tunneled-through-usb4-to-usb4-host-routers > > > > - Integrated Thunderbolt PCIe root ports before Alder Lake do not > > have the usb4-host-interface ACPI property. Identify those with > > their PCI IDs instead. > > > > 2) If a root port does not have integrated Thunderbolt capabilities, but > > has the ExternalFacingPort ACPI property, that means the manufacturer > > has opted to use a discrete Thunderbolt host controller that is > > built into the computer. > > > > This host controller can be identified by virtue of being located > > directly below an external-facing root port that lacks integrated > > Thunderbolt. Label it as trusted and fixed. > > > > Everything downstream from it is untrusted and removable. > > > > The ExternalFacingPort ACPI property is described here: > > Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports > > > > Suggested-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > > Signed-off-by: Esther Shimanovich <eshimanovich@xxxxxxxxxxxx> > > Tested-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > > Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > > --- > > While working with devices that have discrete Thunderbolt chips, I > > noticed that their internal TBT chips are inaccurately labeled as > > untrusted and removable. > > > > I've observed that this issue impacts all computers with internal, > > discrete Intel JHL Thunderbolt chips, such as JHL6240, JHL6340, JHL6540, > > and JHL7540, across multiple device manufacturers such as Lenovo, Dell, > > and HP. > > > > This affects the execution of any downstream security policy that > > relies on the "untrusted" or "removable" flags. > > > > I initially submitted a quirk to resolve this, which was too small in > > scope, and after some discussion, Mika proposed a more thorough fix: > > https://lore.kernel.org/lkml/20240510052616.GC4162345@xxxxxxxxxxxxxxxxxx > > I refactored it and am submitting as a new patch. > > My apologies on my delayed response, I've been OOO a while. > > I had a test with this patch on an AMD Phoenix system on top of 6.11-rc4. I > am noticing that with it, it's now flagging an internal PCI host bridge as > untrusted. I added some extra debugging and it falls through to the last > case of pcie_is_tunneled() where it returns true. > > Here is the topology of the system: > > -[0000:00]-+-00.0 > +-00.2 > +-01.0 > +-01.3-[01]----00.0 > +-02.0 > +-02.1-[02]----00.0 > +-02.4-[03]----00.0 > +-03.0 > +-03.1-[04-62]-- > +-04.0 > +-04.1-[63-c1]-- > +-08.0 > +-08.1-[c2]--+-00.0 > | +-00.1 > | +-00.2 > | +-00.3 > | +-00.4 > | +-00.5 > | +-00.6 > | \-00.7 > +-08.2-[c3]--+-00.0 > | \-00.1 > +-08.3-[c4]--+-00.0 > | +-00.3 > | +-00.4 > | +-00.5 > | \-00.6 > +-14.0 > +-14.3 > +-18.0 > +-18.1 > +-18.2 > +-18.3 > +-18.4 > +-18.5 > +-18.6 > \-18.7 > > Here are the details of all devices on the system: > > 00:00.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14e8] > 00:00.2 IOMMU [0806]: Advanced Micro Devices, Inc. [AMD] Device [1022:14e9] > 00:01.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ea] > 00:01.3 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ee] > 00:02.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ea] > 00:02.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ee] > 00:02.4 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ee] > 00:03.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ea] > 00:03.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h > USB4/Thunderbolt PCIe tunnel [1022:14ef] > 00:04.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ea] > 00:04.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h > USB4/Thunderbolt PCIe tunnel [1022:14ef] > 00:08.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14ea] > 00:08.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14eb] > 00:08.2 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14eb] > 00:08.3 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14eb] > 00:14.0 SMBus [0c05]: Advanced Micro Devices, Inc. [AMD] FCH SMBus > Controller [1022:790b] (rev 71) > 00:14.3 ISA bridge [0601]: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge > [1022:790e] (rev 51) > 00:18.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f0] > 00:18.1 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f1] > 00:18.2 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f2] > 00:18.3 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f3] > 00:18.4 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f4] > 00:18.5 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f5] > 00:18.6 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f6] > 00:18.7 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device > [1022:14f7] > 01:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. > RTL8111/8168/8211/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] > (rev 1c) > 02:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS5261 PCI > Express Card Reader [10ec:5261] (rev 01) > 03:00.0 Non-Volatile memory controller [0108]: Samsung Electronics Co Ltd > NVMe SSD Controller PM9A1/PM9A3/980PRO [144d:a80a] > c2:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. > [AMD/ATI] Phoenix1 [1002:15bf] (rev 03) > c2:00.1 Audio device [0403]: Advanced Micro Devices, Inc. [AMD/ATI] > Rembrandt Radeon High Definition Audio Controller [1002:1640] > c2:00.2 Encryption controller [1080]: Advanced Micro Devices, Inc. [AMD] > Family 19h (Model 74h) CCP/PSP 3.0 Device [1022:15c7] > c2:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device > [1022:15b9] > c2:00.4 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device > [1022:15ba] > c2:00.5 Multimedia controller [0480]: Advanced Micro Devices, Inc. [AMD] > ACP/ACP3X/ACP6x Audio Coprocessor [1022:15e2] (rev 63) > c2:00.6 Audio device [0403]: Advanced Micro Devices, Inc. [AMD] Family > 17h/19h HD Audio Controller [1022:15e3] > c2:00.7 Signal processing controller [1180]: Advanced Micro Devices, Inc. > [AMD] Device [1022:164a] > c3:00.0 Non-Essential Instrumentation [1300]: Advanced Micro Devices, Inc. > [AMD] Device [1022:14ec] > c3:00.1 Signal processing controller [1180]: Advanced Micro Devices, Inc. > [AMD] AMD IPU Device [1022:1502] > c4:00.0 Non-Essential Instrumentation [1300]: Advanced Micro Devices, Inc. > [AMD] Device [1022:14ec] > c4:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device > [1022:15c0] > c4:00.4 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device > [1022:15c1] > c4:00.5 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink > Sardine USB4/Thunderbolt NHI controller #1 [1022:1668] > c4:00.6 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink > Sardine USB4/Thunderbolt NHI controller #2 [1022:1669] > > Here's the snippet from the kernel log with the patch in place. You can see > it flagged 00:02.0 as untrusted and removable, but it definitely isn't. Is it marked as ExternalFacingPort in the ACPI tables?