[PATCH] crypto: ccp: sev-dev-tsm: bail out early when pdev->bus is NULL

[PATCH] crypto: ccp: sev-dev-tsm: bail out early when pdev->bus is NULL

[Stepan Ionichev]

dsm_create() initially checks pdev->bus when computing segment_id:

	u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;

But the next two lines unconditionally dereference pdev->bus via
pcie_find_root_port() and especially pci_dev_id(pdev), which expands
to PCI_DEVID(dev->bus->number, dev->devfn). If pdev->bus is in fact
NULL, segment_id is initialised to 0 but the very next statement
crashes the kernel.

smatch flags this:

  drivers/crypto/ccp/sev-dev-tsm.c:253 dsm_create() error: we
    previously assumed 'pdev->bus' could be null (see line 251)

Make the NULL handling consistent: if pdev->bus is NULL the device
has no PCI context to work with and SEV TIO setup cannot proceed,
so return -ENODEV before any of the bus-dependent lookups. The
remaining initialisation now runs only on the path where pdev->bus
is known to be valid.

No change for callers where pdev->bus is non-NULL, which is the
only case where dsm_create() did meaningful work before this change.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/crypto/ccp/sev-dev-tsm.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev-tsm.c b/drivers/crypto/ccp/sev-dev-tsm.c
index b07ae529b..f303d8f55 100644
--- a/drivers/crypto/ccp/sev-dev-tsm.c
+++ b/drivers/crypto/ccp/sev-dev-tsm.c
@@ -248,12 +248,19 @@ static void dsm_remove(struct pci_tsm *tsm)
 static int dsm_create(struct tio_dsm *dsm)
 {
 	struct pci_dev *pdev = dsm->tsm.base_tsm.pdev;
-	u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;
-	struct pci_dev *rootport = pcie_find_root_port(pdev);
-	u16 device_id = pci_dev_id(pdev);
+	struct pci_dev *rootport;
+	u8 segment_id;
+	u16 device_id;
 	u16 root_port_id;
 	u32 lnkcap = 0;
 
+	if (!pdev->bus)
+		return -ENODEV;
+
+	segment_id = pci_domain_nr(pdev->bus);
+	rootport = pcie_find_root_port(pdev);
+	device_id = pci_dev_id(pdev);
+
 	if (pci_read_config_dword(rootport, pci_pcie_cap(rootport) + PCI_EXP_LNKCAP,
 				  &lnkcap))
 		return -ENODEV;
-- 
2.43.0

[PATCH v2] crypto: ccp: sev-dev-tsm: bail out early when pdev->bus is NULL

[Stepan Ionichev]

dsm_create() initially checks pdev->bus when computing segment_id:

	u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;

But the next two lines unconditionally dereference pdev->bus via
pcie_find_root_port() and especially pci_dev_id(pdev), which expands
to PCI_DEVID(dev->bus->number, dev->devfn). If pdev->bus is in fact
NULL, segment_id is initialised to 0 but the very next statement
crashes the kernel.

smatch flags this:

  drivers/crypto/ccp/sev-dev-tsm.c:253 dsm_create() error: we
    previously assumed 'pdev->bus' could be null (see line 251)

Make the NULL handling consistent: if pdev->bus is NULL the device
has no PCI context to work with and SEV TIO setup cannot proceed,
so return -ENODEV before any of the bus-dependent lookups. The
remaining initialisation now runs only on the path where pdev->bus
is known to be valid.

No change for callers where pdev->bus is non-NULL, which is the
only case where dsm_create() did meaningful work before this change.

Fixes: 4be423572da1 ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Add Fixes: tag (suggested by Tom Lendacky).
- Cc Alexey Kardashevskiy (original author of the SEV-TIO code).

 drivers/crypto/ccp/sev-dev-tsm.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev-tsm.c b/drivers/crypto/ccp/sev-dev-tsm.c
index b07ae529b..f303d8f55 100644
--- a/drivers/crypto/ccp/sev-dev-tsm.c
+++ b/drivers/crypto/ccp/sev-dev-tsm.c
@@ -248,12 +248,19 @@ static void dsm_remove(struct pci_tsm *tsm)
 static int dsm_create(struct tio_dsm *dsm)
 {
 	struct pci_dev *pdev = dsm->tsm.base_tsm.pdev;
-	u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;
-	struct pci_dev *rootport = pcie_find_root_port(pdev);
-	u16 device_id = pci_dev_id(pdev);
+	struct pci_dev *rootport;
+	u8 segment_id;
+	u16 device_id;
 	u16 root_port_id;
 	u32 lnkcap = 0;

+	if (!pdev->bus)
+		return -ENODEV;
+
+	segment_id = pci_domain_nr(pdev->bus);
+	rootport = pcie_find_root_port(pdev);
+	device_id = pci_dev_id(pdev);
+
 	if (pci_read_config_dword(rootport, pci_pcie_cap(rootport) + PCI_EXP_LNKCAP,
 				  &lnkcap))
 		return -ENODEV;
--
2.43.0

Re: [PATCH] crypto: ccp: sev-dev-tsm: bail out early when pdev->bus is NULL

[Tom Lendacky]

On 5/6/26 21:36, Stepan Ionichev wrote:
> dsm_create() initially checks pdev->bus when computing segment_id:
> 
> 	u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;
> 
> But the next two lines unconditionally dereference pdev->bus via
> pcie_find_root_port() and especially pci_dev_id(pdev), which expands
> to PCI_DEVID(dev->bus->number, dev->devfn). If pdev->bus is in fact
> NULL, segment_id is initialised to 0 but the very next statement
> crashes the kernel.
> 
> smatch flags this:
> 
>   drivers/crypto/ccp/sev-dev-tsm.c:253 dsm_create() error: we
>     previously assumed 'pdev->bus' could be null (see line 251)
> 
> Make the NULL handling consistent: if pdev->bus is NULL the device
> has no PCI context to work with and SEV TIO setup cannot proceed,
> so return -ENODEV before any of the bus-dependent lookups. The
> remaining initialisation now runs only on the path where pdev->bus
> is known to be valid.
> 
> No change for callers where pdev->bus is non-NULL, which is the
> only case where dsm_create() did meaningful work before this change.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

This should have a Fixes: tag, which would have then added the main author
of the file to the email, so adding Alexey Kardashevskiy manually.

@Alexey, please also submit a patch to make yourself a maintainer of the
TIO/TSM support.

Thanks,
Tom

> ---
>  drivers/crypto/ccp/sev-dev-tsm.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev-tsm.c b/drivers/crypto/ccp/sev-dev-tsm.c
> index b07ae529b..f303d8f55 100644
> --- a/drivers/crypto/ccp/sev-dev-tsm.c
> +++ b/drivers/crypto/ccp/sev-dev-tsm.c
> @@ -248,12 +248,19 @@ static void dsm_remove(struct pci_tsm *tsm)
>  static int dsm_create(struct tio_dsm *dsm)
>  {
>  	struct pci_dev *pdev = dsm->tsm.base_tsm.pdev;
> -	u8 segment_id = pdev->bus ? pci_domain_nr(pdev->bus) : 0;
> -	struct pci_dev *rootport = pcie_find_root_port(pdev);
> -	u16 device_id = pci_dev_id(pdev);
> +	struct pci_dev *rootport;
> +	u8 segment_id;
> +	u16 device_id;
>  	u16 root_port_id;
>  	u32 lnkcap = 0;
>  
> +	if (!pdev->bus)
> +		return -ENODEV;
> +
> +	segment_id = pci_domain_nr(pdev->bus);
> +	rootport = pcie_find_root_port(pdev);
> +	device_id = pci_dev_id(pdev);
> +
>  	if (pci_read_config_dword(rootport, pci_pcie_cap(rootport) + PCI_EXP_LNKCAP,
>  				  &lnkcap))
>  		return -ENODEV;