On Fri, Aug 23, 2024 at 04:53:16PM +0000, Esther Shimanovich wrote:
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> +static bool pcie_has_usb4_host_interface(struct pci_dev *pdev)
> +{
> + struct fwnode_handle *fwnode;
> +
> + /*
> + * For USB4, the tunneled PCIe root or downstream ports are marked
> + * with the "usb4-host-interface" ACPI property, so we look for
> + * that first. This should cover most cases.
> + */
> + fwnode = fwnode_find_reference(dev_fwnode(&pdev->dev),
> + "usb4-host-interface", 0);
This is all ACPI only, so it should either be #ifdef'ed to CONFIG_ACPI
or moved to drivers/pci/pci-acpi.c.
Alternatively, it could be moved to arch/x86/pci/ because ACPI can also
be enabled on arm64 or riscv but the issue seems to only affect x86.
> static void set_pcie_untrusted(struct pci_dev *dev)
> {
> - struct pci_dev *parent;
> + struct pci_dev *parent = pci_upstream_bridge(dev);
>
> + if (!parent)
> + return;
> /*
> - * If the upstream bridge is untrusted we treat this device
> + * If the upstream bridge is untrusted we treat this device as
> * untrusted as well.
> */
> - parent = pci_upstream_bridge(dev);
> - if (parent && (parent->untrusted || parent->external_facing))
> + if (parent->untrusted)
> dev->untrusted = true;
> +
> + if (pcie_is_tunneled(dev)) {
> + pci_dbg(dev, "marking as untrusted\n");
> + dev->untrusted = true;
> + }
> }
I think you want to return in the "if (parent->untrusted)" case
because there's no need to double-check pcie_is_tunneled(dev)
if you've already determined that the device is untrusted.
> static void pci_set_removable(struct pci_dev *dev)
> {
> struct pci_dev *parent = pci_upstream_bridge(dev);
>
> + if (!parent)
> + return;
> /*
> - * We (only) consider everything downstream from an external_facing
> + * We (only) consider everything tunneled below an external_facing
> * device to be removable by the user. We're mainly concerned with
> * consumer platforms with user accessible thunderbolt ports that are
> * vulnerable to DMA attacks, and we expect those ports to be marked by
> @@ -1657,9 +1784,13 @@ static void pci_set_removable(struct pci_dev *dev)
> * accessible to user / may not be removed by end user, and thus not
> * exposed as "removable" to userspace.
> */
> - if (parent &&
> - (parent->external_facing || dev_is_removable(&parent->dev)))
> + if (dev_is_removable(&parent->dev))
> + dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
> +
> + if (pcie_is_tunneled(dev)) {
> + pci_dbg(dev, "marking as removable\n");
> dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
> + }
> }
Same here, return in the "if (dev_is_removable(&parent->dev))" case.
Thanks,
Lukas
