[PATCH] crypto: ccp - Run the destroy callbacks when psp_init() fails

[PATCH] crypto: ccp - Run the destroy callbacks when psp_init() fails

[Kim Phillips]

psp_init() brings up SEV, TEE, SFS, platform access, DBC and HSTI in
turn, returning on the first failure.  psp_dev_init() then clears
sp->psp_data, and sp_init() ignores its return value, so the device stays
bound.  At unbind, sp_destroy() calls psp_dev_destroy(), which returns
early on the now NULL psp_data, and no subsystem destroy callback ever
runs.

Whatever came up before the failing subsystem is therefore abandoned.  Not
everything it owns is devres-managed: tee_dev_init() allocates its ring
with __get_free_pages() and it is freed only by tee_destroy_ring(), so a
failure anywhere after TEE leaks the ring.  DBC and SFS are worse than a
leak.  Both register a misc device during init, and both deregister it
from their destroy callback, so a psp_init_hsti() failure leaves a live
/dev node whose backing state devres frees at unbind.

Call psp_dev_destroy() on that path instead of open-coding the IRQ and
master device teardown.  Every destroy callback returns early when its
subsystem pointer is NULL, and psp is devm_kzalloc()'d, so unwinding a
partial init is safe.  This also fixes the teardown order: the callbacks
now run before the master device is cleared, which matters because
__sev_platform_shutdown_locked() reaches the device through psp_master.

sev_dev_destroy() is reachable here without interrupts having been enabled
yet, but it does not issue a command: firmware INIT happens later in
sev_pci_init(), so the state is still SEV_STATE_UNINIT and
__sev_platform_shutdown_locked() returns without touching the mailbox.

Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
 drivers/crypto/ccp/psp-dev.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index b14ce51065d5..4eb49eff9590 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -282,7 +282,7 @@ int psp_dev_init(struct sp_device *sp)
 
 	ret = psp_init(psp);
 	if (ret)
-		goto e_irq;
+		goto e_destroy;
 
 	/* Enable interrupt */
 	iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
@@ -291,11 +291,15 @@ int psp_dev_init(struct sp_device *sp)
 
 	return 0;
 
-e_irq:
-	if (sp->clear_psp_master_device)
-		sp->clear_psp_master_device(sp);
-
-	sp_free_psp_irq(psp->sp, psp);
+e_destroy:
+	/*
+	 * Subsystems that came up before the failure own resources devres
+	 * cannot reclaim - TEE's ring pages, the DBC and SFS misc devices -
+	 * and psp_data is cleared below, which puts them out of reach of
+	 * sp_destroy(). Each destroy no-ops on a subsystem that never
+	 * initialized, and this also drops the IRQ and master device.
+	 */
+	psp_dev_destroy(sp);
 e_err:
 	sp->psp_data = NULL;
 
-- 
2.43.0

Re: [PATCH] crypto: ccp - Run the destroy callbacks when psp_init() fails

[Kim Phillips]

Hi all,

Please ignore this patch.

Thank you,

Kim

On 10/21/26 4:57 PM, Kim Phillips wrote:
> psp_init() brings up SEV, TEE, SFS, platform access, DBC and HSTI in
> turn, returning on the first failure.  psp_dev_init() then clears
> sp->psp_data, and sp_init() ignores its return value, so the device stays
> bound.  At unbind, sp_destroy() calls psp_dev_destroy(), which returns
> early on the now NULL psp_data, and no subsystem destroy callback ever
> runs.
> 
> Whatever came up before the failing subsystem is therefore abandoned.  Not
> everything it owns is devres-managed: tee_dev_init() allocates its ring
> with __get_free_pages() and it is freed only by tee_destroy_ring(), so a
> failure anywhere after TEE leaks the ring.  DBC and SFS are worse than a
> leak.  Both register a misc device during init, and both deregister it
> from their destroy callback, so a psp_init_hsti() failure leaves a live
> /dev node whose backing state devres frees at unbind.
> 
> Call psp_dev_destroy() on that path instead of open-coding the IRQ and
> master device teardown.  Every destroy callback returns early when its
> subsystem pointer is NULL, and psp is devm_kzalloc()'d, so unwinding a
> partial init is safe.  This also fixes the teardown order: the callbacks
> now run before the master device is cleared, which matters because
> __sev_platform_shutdown_locked() reaches the device through psp_master.
> 
> sev_dev_destroy() is reachable here without interrupts having been enabled
> yet, but it does not issue a command: firmware INIT happens later in
> sev_pci_init(), so the state is still SEV_STATE_UNINIT and
> __sev_platform_shutdown_locked() returns without touching the mailbox.
> 
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
> ---
>   drivers/crypto/ccp/psp-dev.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index b14ce51065d5..4eb49eff9590 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -282,7 +282,7 @@ int psp_dev_init(struct sp_device *sp)
>   
>   	ret = psp_init(psp);
>   	if (ret)
> -		goto e_irq;
> +		goto e_destroy;
>   
>   	/* Enable interrupt */
>   	iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
> @@ -291,11 +291,15 @@ int psp_dev_init(struct sp_device *sp)
>   
>   	return 0;
>   
> -e_irq:
> -	if (sp->clear_psp_master_device)
> -		sp->clear_psp_master_device(sp);
> -
> -	sp_free_psp_irq(psp->sp, psp);
> +e_destroy:
> +	/*
> +	 * Subsystems that came up before the failure own resources devres
> +	 * cannot reclaim - TEE's ring pages, the DBC and SFS misc devices -
> +	 * and psp_data is cleared below, which puts them out of reach of
> +	 * sp_destroy(). Each destroy no-ops on a subsystem that never
> +	 * initialized, and this also drops the IRQ and master device.
> +	 */
> +	psp_dev_destroy(sp);
>   e_err:
>   	sp->psp_data = NULL;
>