diff mbox series

[10/12] PCI/CMA: Reauthenticate devices on reset and resume

Message ID e71af6b0bf695694d4a6de2c44356fe4fda97fea.1695921657.git.lukas@wunner.de (mailing list archive)
State Changes Requested
Delegated to: Bjorn Helgaas
Headers show
Series PCI device authentication | expand

Commit Message

Lukas Wunner Sept. 28, 2023, 5:32 p.m. UTC
CMA-SPDM state is lost when a device undergoes a Conventional Reset.
(But not a Function Level Reset, PCIe r6.1 sec 6.6.2.)  A D3cold to D0
transition implies a Conventional Reset (PCIe r6.1 sec 5.8).

Thus, reauthenticate devices on resume from D3cold and on recovery from
a Secondary Bus Reset or DPC-induced Hot Reset.

The requirement to reauthenticate devices on resume from system sleep
(and in the future reestablish IDE encryption) is the reason why SPDM
needs to be in-kernel:  During ->resume_noirq, which is the first phase
after system sleep, the PCI core walks down the hierarchy, puts each
device in D0, restores its config space and invokes the driver's
->resume_noirq callback.  The driver is afforded the right to access the
device already during this phase.

To retain this usage model in the face of authentication and encryption,
CMA-SPDM reauthentication and IDE reestablishment must happen during the
->resume_noirq phase, before the driver's first access to the device.
The driver is thus afforded seamless authenticated and encrypted access
until the last moment before suspend and from the first moment after
resume.

During the ->resume_noirq phase, device interrupts are not yet enabled.
It is thus impossible to defer CMA-SPDM reauthentication to a user space
component on an attached disk or on the network, making an in-kernel
SPDM implementation mandatory.

The same catch-22 exists on recovery from a Conventional Reset:  A user
space SPDM implementation might live on a device which underwent reset,
rendering its execution impossible.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/cma.c        | 10 ++++++++++
 drivers/pci/pci-driver.c |  1 +
 drivers/pci/pci.c        | 12 ++++++++++--
 drivers/pci/pci.h        |  5 +++++
 drivers/pci/pcie/err.c   |  3 +++
 include/linux/pci.h      |  1 +
 6 files changed, 30 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron Oct. 3, 2023, 3:10 p.m. UTC | #1
On Thu, 28 Sep 2023 19:32:40 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> CMA-SPDM state is lost when a device undergoes a Conventional Reset.
> (But not a Function Level Reset, PCIe r6.1 sec 6.6.2.)  A D3cold to D0
> transition implies a Conventional Reset (PCIe r6.1 sec 5.8).
> 
> Thus, reauthenticate devices on resume from D3cold and on recovery from
> a Secondary Bus Reset or DPC-induced Hot Reset.
> 
> The requirement to reauthenticate devices on resume from system sleep
> (and in the future reestablish IDE encryption) is the reason why SPDM
> needs to be in-kernel:  During ->resume_noirq, which is the first phase
> after system sleep, the PCI core walks down the hierarchy, puts each
> device in D0, restores its config space and invokes the driver's
> ->resume_noirq callback.  The driver is afforded the right to access the  
> device already during this phase.
> 
> To retain this usage model in the face of authentication and encryption,
> CMA-SPDM reauthentication and IDE reestablishment must happen during the
> ->resume_noirq phase, before the driver's first access to the device.  
> The driver is thus afforded seamless authenticated and encrypted access
> until the last moment before suspend and from the first moment after
> resume.
> 
> During the ->resume_noirq phase, device interrupts are not yet enabled.
> It is thus impossible to defer CMA-SPDM reauthentication to a user space
> component on an attached disk or on the network, making an in-kernel
> SPDM implementation mandatory.
> 
> The same catch-22 exists on recovery from a Conventional Reset:  A user
> space SPDM implementation might live on a device which underwent reset,
> rendering its execution impossible.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/pci/cma.c        | 10 ++++++++++
>  drivers/pci/pci-driver.c |  1 +
>  drivers/pci/pci.c        | 12 ++++++++++--
>  drivers/pci/pci.h        |  5 +++++
>  drivers/pci/pcie/err.c   |  3 +++
>  include/linux/pci.h      |  1 +
>  6 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> index 012190c54ab6..89d23fdc37ec 100644
> --- a/drivers/pci/cma.c
> +++ b/drivers/pci/cma.c
> @@ -71,6 +71,16 @@ void pci_cma_init(struct pci_dev *pdev)
>  	}
>  
>  	rc = spdm_authenticate(pdev->spdm_state);
> +	if (rc != -EPROTONOSUPPORT)
> +		pdev->cma_capable = true;
This is the blob that I think wants pulling forwards
to earlier patch so that rc = 
isn't left hanging.

> +}
> +
> +int pci_cma_reauthenticate(struct pci_dev *pdev)
> +{
> +	if (!pdev->cma_capable)
> +		return -ENOTTY;
> +
> +	return spdm_authenticate(pdev->spdm_state);

If authenticate failed why did we leave spdm_state around?
That feels like a corner case in the earlier patch that needs
documentation.  I can see maybe certs not provisioned yet would
be a valid reason or an intermittent fault (solved by reset)
but in those cases we'd want to try again on reset anyway...

>  }
>  
>  void pci_cma_destroy(struct pci_dev *pdev)
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index a79c110c7e51..b5d47eefe8df 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -568,6 +568,7 @@ static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
>  	pci_pm_power_up_and_verify_state(pci_dev);
>  	pci_restore_state(pci_dev);
>  	pci_pme_restore(pci_dev);
> +	pci_cma_reauthenticate(pci_dev);
>  }
>  
>  static void pci_pm_bridge_power_up_actions(struct pci_dev *pci_dev)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 59c01d68c6d5..0f36e6082579 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5248,8 +5248,16 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
>  
>  	rc = pci_dev_reset_slot_function(dev, probe);
>  	if (rc != -ENOTTY)
> -		return rc;
> -	return pci_parent_bus_reset(dev, probe);
> +		goto done;
> +
> +	rc = pci_parent_bus_reset(dev, probe);
> +
> +done:
> +	/* CMA-SPDM state is lost upon a Conventional Reset */
> +	if (!probe)
> +		pci_cma_reauthenticate(dev);
> +
> +	return rc;
>  }
>  
>  void pci_dev_lock(struct pci_dev *dev)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 6c4755a2c91c..71092ccf4fbd 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -325,11 +325,16 @@ static inline void pci_doe_disconnected(struct pci_dev *pdev) { }
>  #ifdef CONFIG_PCI_CMA
>  void pci_cma_init(struct pci_dev *pdev);
>  void pci_cma_destroy(struct pci_dev *pdev);
> +int pci_cma_reauthenticate(struct pci_dev *pdev);
>  struct x509_certificate;
>  int pci_cma_validate(struct device *dev, struct x509_certificate *leaf_cert);
>  #else
>  static inline void pci_cma_init(struct pci_dev *pdev) { }
>  static inline void pci_cma_destroy(struct pci_dev *pdev) { }
> +static inline int pci_cma_reauthenticate(struct pci_dev *pdev)
> +{
> +	return -ENOTTY;
> +}
>  #endif
>  
>  /**
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 59c90d04a609..4783bd907b54 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -122,6 +122,9 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
>  	pci_ers_result_t vote, *result = data;
>  	const struct pci_error_handlers *err_handler;
>  
> +	/* CMA-SPDM state is lost upon a Conventional Reset */
> +	pci_cma_reauthenticate(dev);
> +
>  	device_lock(&dev->dev);
>  	pdrv = dev->driver;
>  	if (!pdrv ||
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 0c0123317df6..2bc11d8b567e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -519,6 +519,7 @@ struct pci_dev {
>  #endif
>  #ifdef CONFIG_PCI_CMA
>  	struct spdm_state *spdm_state;	/* Security Protocol and Data Model */
> +	unsigned int	cma_capable:1;	/* Authentication supported */
Also this should I think move to the earlier patch where we know if it is supported
even though we don't use it until here.

>  #endif
>  	u16		acs_cap;	/* ACS Capability offset */
>  	phys_addr_t	rom;		/* Physical address if not from BAR */
diff mbox series

Patch

diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
index 012190c54ab6..89d23fdc37ec 100644
--- a/drivers/pci/cma.c
+++ b/drivers/pci/cma.c
@@ -71,6 +71,16 @@  void pci_cma_init(struct pci_dev *pdev)
 	}
 
 	rc = spdm_authenticate(pdev->spdm_state);
+	if (rc != -EPROTONOSUPPORT)
+		pdev->cma_capable = true;
+}
+
+int pci_cma_reauthenticate(struct pci_dev *pdev)
+{
+	if (!pdev->cma_capable)
+		return -ENOTTY;
+
+	return spdm_authenticate(pdev->spdm_state);
 }
 
 void pci_cma_destroy(struct pci_dev *pdev)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a79c110c7e51..b5d47eefe8df 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -568,6 +568,7 @@  static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
 	pci_pm_power_up_and_verify_state(pci_dev);
 	pci_restore_state(pci_dev);
 	pci_pme_restore(pci_dev);
+	pci_cma_reauthenticate(pci_dev);
 }
 
 static void pci_pm_bridge_power_up_actions(struct pci_dev *pci_dev)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 59c01d68c6d5..0f36e6082579 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5248,8 +5248,16 @@  static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
 
 	rc = pci_dev_reset_slot_function(dev, probe);
 	if (rc != -ENOTTY)
-		return rc;
-	return pci_parent_bus_reset(dev, probe);
+		goto done;
+
+	rc = pci_parent_bus_reset(dev, probe);
+
+done:
+	/* CMA-SPDM state is lost upon a Conventional Reset */
+	if (!probe)
+		pci_cma_reauthenticate(dev);
+
+	return rc;
 }
 
 void pci_dev_lock(struct pci_dev *dev)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6c4755a2c91c..71092ccf4fbd 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -325,11 +325,16 @@  static inline void pci_doe_disconnected(struct pci_dev *pdev) { }
 #ifdef CONFIG_PCI_CMA
 void pci_cma_init(struct pci_dev *pdev);
 void pci_cma_destroy(struct pci_dev *pdev);
+int pci_cma_reauthenticate(struct pci_dev *pdev);
 struct x509_certificate;
 int pci_cma_validate(struct device *dev, struct x509_certificate *leaf_cert);
 #else
 static inline void pci_cma_init(struct pci_dev *pdev) { }
 static inline void pci_cma_destroy(struct pci_dev *pdev) { }
+static inline int pci_cma_reauthenticate(struct pci_dev *pdev)
+{
+	return -ENOTTY;
+}
 #endif
 
 /**
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 59c90d04a609..4783bd907b54 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -122,6 +122,9 @@  static int report_slot_reset(struct pci_dev *dev, void *data)
 	pci_ers_result_t vote, *result = data;
 	const struct pci_error_handlers *err_handler;
 
+	/* CMA-SPDM state is lost upon a Conventional Reset */
+	pci_cma_reauthenticate(dev);
+
 	device_lock(&dev->dev);
 	pdrv = dev->driver;
 	if (!pdrv ||
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0c0123317df6..2bc11d8b567e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -519,6 +519,7 @@  struct pci_dev {
 #endif
 #ifdef CONFIG_PCI_CMA
 	struct spdm_state *spdm_state;	/* Security Protocol and Data Model */
+	unsigned int	cma_capable:1;	/* Authentication supported */
 #endif
 	u16		acs_cap;	/* ACS Capability offset */
 	phys_addr_t	rom;		/* Physical address if not from BAR */