public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tom Lendacky <thomas.lendacky@amd.com>
To: Borislav Petkov <bp@kernel.org>, Ashish Kalra <ashish.kalra@amd.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	linux-crypto@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	"Borislav Petkov (AMD)" <bp@alien8.de>,
	stable@kernel.org
Subject: Re: [PATCH] crypto: ccp - Always pass in an error pointer to __sev_platform_shutdown_locked()
Date: Mon, 8 Sep 2025 12:07:23 -0500	[thread overview]
Message-ID: <7bcb1ba5-e655-438e-faab-a1c99637f78a@amd.com> (raw)
In-Reply-To: <20250906122145.29784-1-bp@kernel.org>

On 9/6/25 07:21, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
> 
> When
> 
>   9770b428b1a2 ("crypto: ccp - Move dev_info/err messages for SEV/SNP init and shutdown")
> 
> moved the error messages dumping so that they don't need to be issued by
> the callers, it missed the case where __sev_firmware_shutdown() calls
> __sev_platform_shutdown_locked() with a NULL argument which leads to
> a NULL ptr deref on the shutdown path, during suspend to disk:
> 
>   #PF: supervisor read access in kernel mode
>   #PF: error_code(0x0000) - not-present page
>   PGD 0 P4D 0
>   Oops: Oops: 0000 [#1] SMP NOPTI
>   CPU: 0 UID: 0 PID: 983 Comm: hib.sh Not tainted 6.17.0-rc4+ #1 PREEMPT(voluntary)
>   Hardware name: Supermicro Super Server/H12SSL-i, BIOS 2.5 09/08/2022
>   RIP: 0010:__sev_platform_shutdown_locked.cold+0x0/0x21 [ccp]
> 
> That rIP is:
> 
>   00000000000006fd <__sev_platform_shutdown_locked.cold>:
>    6fd:   8b 13                   mov    (%rbx),%edx
>    6ff:   48 8b 7d 00             mov    0x0(%rbp),%rdi
>    703:   89 c1                   mov    %eax,%ecx
> 
>   Code: 74 05 31 ff 41 89 3f 49 8b 3e 89 ea 48 c7 c6 a0 8e 54 a0 41 bf 92 ff ff ff e8 e5 2e 09 e1 c6 05 2a d4 38 00 01 e9 26 af ff ff <8b> 13 48 8b 7d 00 89 c1 48 c7 c6 18 90 54 a0 89 44 24 04 e8 c1 2e
>   RSP: 0018:ffffc90005467d00 EFLAGS: 00010282
>   RAX: 00000000ffffff92 RBX: 0000000000000000 RCX: 0000000000000000
>   			     ^^^^^^^^^^^^^^^^
> and %rbx is nice and clean.
> 
>   Call Trace:
>    <TASK>
>    __sev_firmware_shutdown.isra.0
>    sev_dev_destroy
>    psp_dev_destroy
>    sp_destroy
>    pci_device_shutdown
>    device_shutdown
>    kernel_power_off
>    hibernate.cold
>    state_store
>    kernfs_fop_write_iter
>    vfs_write
>    ksys_write
>    do_syscall_64
>    entry_SYSCALL_64_after_hwframe
> 
> Pass in a pointer to the function-local error var in the caller.
> 
> With that addressed, suspending the ccp shows the error properly at
> least:
> 
>   ccp 0000:47:00.1: sev command 0x2 timed out, disabling PSP
>   ccp 0000:47:00.1: SEV: failed to SHUTDOWN error 0x0, rc -110
>   SEV-SNP: Leaking PFN range 0x146800-0x146a00
>   SEV-SNP: PFN 0x146800 unassigned, dumping non-zero entries in 2M PFN region: [0x146800 - 0x146a00]
>   ...
>   ccp 0000:47:00.1: SEV-SNP firmware shutdown failed, rc -16, error 0x0
>   ACPI: PM: Preparing to enter system sleep state S5
>   kvm: exiting hardware virtualization
>   reboot: Power down
> 
> Btw, this driver is crying to be cleaned up to pass in a proper I/O
> struct which can be used to store information between the different
> functions, otherwise stuff like that will happen in the future again.
> 
> Fixes: 9770b428b1a2 ("crypto: ccp - Move dev_info/err messages for SEV/SNP init and shutdown")
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Cc: <stable@kernel.org>

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  drivers/crypto/ccp/sev-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index e058ba027792..9f5ccc1720cb 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -2430,7 +2430,7 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic)
>  {
>  	int error;
>  
> -	__sev_platform_shutdown_locked(NULL);
> +	__sev_platform_shutdown_locked(&error);
>  
>  	if (sev_es_tmr) {
>  		/*

  parent reply	other threads:[~2025-09-08 17:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-06 12:21 [PATCH] crypto: ccp - Always pass in an error pointer to __sev_platform_shutdown_locked() Borislav Petkov
2025-09-06 21:15 ` Kalra, Ashish
2025-09-08 17:07 ` Tom Lendacky [this message]
2025-09-13  4:31 ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7bcb1ba5-e655-438e-faab-a1c99637f78a@amd.com \
    --to=thomas.lendacky@amd.com \
    --cc=ashish.kalra@amd.com \
    --cc=bp@alien8.de \
    --cc=bp@kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox