On Sun, 30 Jun 2024 21:46:00 +0200
Lukas Wunner <lukas@xxxxxxxxx> wrote:
> The PCI core has just been amended to authenticate CMA-capable devices
> on enumeration and store the result in an "authenticated" bit in struct
> pci_dev->spdm_state.
>
> Expose the bit to user space through an eponymous sysfs attribute.
>
> Allow user space to trigger reauthentication (e.g. after it has updated
> the CMA keyring) by writing to the sysfs attribute.
>
> Implement the attribute in the SPDM library so that other bus types
> besides PCI may take advantage of it. They just need to add
> spdm_attr_group to the attribute groups of their devices and amend the
> dev_to_spdm_state() helper which retrieves the spdm_state for a given
> device.
>
> The helper may return an ERR_PTR if it couldn't be determined whether
> SPDM is supported by the device. The sysfs attribute is visible in that
> case but returns an error on access. This prevents downgrade attacks
> where an attacker disturbs memory allocation or DOE communication
> in order to create the appearance that SPDM is unsupported.
>
> Subject to further discussion, a future commit might add a user-defined
> policy to forbid driver binding to devices which failed authentication,
> similar to the "authorized" attribute for USB.
>
> Alternatively, authentication success might be signaled to user space
> through a uevent, whereupon it may bind a (blacklisted) driver.
> A uevent signaling authentication failure might similarly cause user
> space to unbind or outright remove the potentially malicious device.
>
> Traffic from devices which failed authentication could also be filtered
> through ACS I/O Request Blocking Enable (PCIe r6.2 sec 7.7.11.3) or
> through Link Disable (PCIe r6.2 sec 7.5.3.7). Unlike an IOMMU, that
> will not only protect the host, but also prevent malicious peer-to-peer
> traffic to other devices.
>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
One question on a bit of error path cleanup that I can't immediately see
the reason for.
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index 34bb8f232799..0f94c4ed719e 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -694,6 +694,7 @@ void pci_doe_init(struct pci_dev *pdev)
> if (IS_ERR(doe_mb)) {
> pci_err(pdev, "[%x] failed to create mailbox: %ld\n",
> offset, PTR_ERR(doe_mb));
> + pci_cma_disable(pdev);
Why? pci_cma_init() is currently called after pci_doe_init() so I don't
see why we need to disable here. If we want a default of disabled, do that
before calling pci_doe_init() rather than in the error paths
1) Set default to disabled.
2) pci_doe_init()
3) pci_cma_init() - now not disabled.
> continue;
> }
>
> @@ -702,6 +703,7 @@ void pci_doe_init(struct pci_dev *pdev)
> pci_err(pdev, "[%x] failed to insert mailbox: %d\n",
> offset, rc);
> pci_doe_destroy_mb(doe_mb);
> + pci_cma_disable(pdev);
> }
> }
> }
