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

[Kim Phillips <kim.phillips@amd.com>]

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;
>