* Re: [PATCH] crypto: ccp - Always pass in an error pointer to __sev_platform_shutdown_locked()
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
2025-09-13 4:31 ` Herbert Xu
2 siblings, 0 replies; 4+ messages in thread
From: Kalra, Ashish @ 2025-09-06 21:15 UTC (permalink / raw)
To: Borislav Petkov
Cc: Tom Lendacky, Herbert Xu, linux-crypto, LKML,
Borislav Petkov (AMD), stable
On 9/6/2025 7:21 AM, 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>
> ---
> 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) {
> /*
Reviewed-by: Ashish Kalra <ashish.kalra@amd.com>
Thanks,
Ashish
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] crypto: ccp - Always pass in an error pointer to __sev_platform_shutdown_locked()
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
2025-09-13 4:31 ` Herbert Xu
2 siblings, 0 replies; 4+ messages in thread
From: Tom Lendacky @ 2025-09-08 17:07 UTC (permalink / raw)
To: Borislav Petkov, Ashish Kalra
Cc: Herbert Xu, linux-crypto, LKML, Borislav Petkov (AMD), stable
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) {
> /*
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] crypto: ccp - Always pass in an error pointer to __sev_platform_shutdown_locked()
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
@ 2025-09-13 4:31 ` Herbert Xu
2 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2025-09-13 4:31 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ashish Kalra, Tom Lendacky, linux-crypto, LKML,
Borislav Petkov (AMD), stable
On Sat, Sep 06, 2025 at 02:21:45PM +0200, 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>
> ---
> drivers/crypto/ccp/sev-dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 4+ messages in thread