Re: [PATCH v3] PCI: Detect and trust built-in Thunderbolt chips

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/22/2024 10:53, Mika Westerberg wrote:
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?

No; it doesn't have an ACPI companion device.




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux