diff mbox series

crypto: ccp - Fix null pointer dereference in __sev_snp_shutdown_locked

Message ID 20240603151212.18342-1-kim.phillips@amd.com (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series crypto: ccp - Fix null pointer dereference in __sev_snp_shutdown_locked | expand

Commit Message

Kim Phillips June 3, 2024, 3:12 p.m. UTC
Another DEBUG_TEST_DRIVER_REMOVE induced splat found, this time
in __sev_snp_shutdown_locked().

[   38.625613] ccp 0000:55:00.5: enabling device (0000 -> 0002)
[   38.633022] ccp 0000:55:00.5: sev enabled
[   38.637498] ccp 0000:55:00.5: psp enabled
[   38.642011] BUG: kernel NULL pointer dereference, address: 00000000000000f0
[   38.645963] #PF: supervisor read access in kernel mode
[   38.645963] #PF: error_code(0x0000) - not-present page
[   38.645963] PGD 0 P4D 0
[   38.645963] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC NOPTI
[   38.645963] CPU: 262 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc1+ #29
[   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
[   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
[   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
[   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
[   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
[   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
[   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
[   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
[   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
[   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
[   38.645963] PKRU: 55555554
[   38.645963] Call Trace:
[   38.645963]  <TASK>
[   38.645963]  ? __die_body+0x6f/0xb0
[   38.645963]  ? __die+0xcc/0xf0
[   38.645963]  ? page_fault_oops+0x330/0x3a0
[   38.645963]  ? save_trace+0x2a5/0x360
[   38.645963]  ? do_user_addr_fault+0x583/0x630
[   38.645963]  ? exc_page_fault+0x81/0x120
[   38.645963]  ? asm_exc_page_fault+0x2b/0x30
[   38.645963]  ? __sev_snp_shutdown_locked+0x2e/0x150
[   38.645963]  __sev_firmware_shutdown+0x349/0x5b0
[   38.645963]  ? pm_runtime_barrier+0x66/0xe0
[   38.645963]  sev_dev_destroy+0x34/0xb0
[   38.645963]  psp_dev_destroy+0x27/0x60
[   38.645963]  sp_destroy+0x39/0x90
[   38.645963]  sp_pci_remove+0x22/0x60
[   38.645963]  pci_device_remove+0x4e/0x110
[   38.645963]  really_probe+0x271/0x4e0
[   38.645963]  __driver_probe_device+0x8f/0x160
[   38.645963]  driver_probe_device+0x24/0x120
[   38.645963]  __driver_attach+0xc7/0x280
[   38.645963]  ? driver_attach+0x30/0x30
[   38.645963]  bus_for_each_dev+0x10d/0x130
[   38.645963]  driver_attach+0x22/0x30
[   38.645963]  bus_add_driver+0x171/0x2b0
[   38.645963]  ? unaccepted_memory_init_kdump+0x20/0x20
[   38.645963]  driver_register+0x67/0x100
[   38.645963]  __pci_register_driver+0x83/0x90
[   38.645963]  sp_pci_init+0x22/0x30
[   38.645963]  sp_mod_init+0x13/0x30
[   38.645963]  do_one_initcall+0xb8/0x290
[   38.645963]  ? sched_clock_noinstr+0xd/0x10
[   38.645963]  ? local_clock_noinstr+0x3e/0x100
[   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
[   38.645963]  ? local_clock+0x1c/0x60
[   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
[   38.645963]  ? sched_clock_noinstr+0xd/0x10
[   38.645963]  ? local_clock_noinstr+0x3e/0x100
[   38.645963]  ? __lock_acquire+0xd90/0xe30
[   38.645963]  ? sched_clock_noinstr+0xd/0x10
[   38.645963]  ? local_clock_noinstr+0x3e/0x100
[   38.645963]  ? __create_object+0x66/0x100
[   38.645963]  ? local_clock+0x1c/0x60
[   38.645963]  ? __create_object+0x66/0x100
[   38.645963]  ? parameq+0x1b/0x90
[   38.645963]  ? parse_one+0x6d/0x1d0
[   38.645963]  ? parse_args+0xd7/0x1f0
[   38.645963]  ? do_initcall_level+0x180/0x180
[   38.645963]  do_initcall_level+0xb0/0x180
[   38.645963]  do_initcalls+0x60/0xa0
[   38.645963]  ? kernel_init+0x1f/0x1d0
[   38.645963]  do_basic_setup+0x41/0x50
[   38.645963]  kernel_init_freeable+0x1ac/0x230
[   38.645963]  ? rest_init+0x1f0/0x1f0
[   38.645963]  kernel_init+0x1f/0x1d0
[   38.645963]  ? rest_init+0x1f0/0x1f0
[   38.645963]  ret_from_fork+0x3d/0x50
[   38.645963]  ? rest_init+0x1f0/0x1f0
[   38.645963]  ret_from_fork_asm+0x11/0x20
[   38.645963]  </TASK>
[   38.645963] Modules linked in:
[   38.645963] CR2: 00000000000000f0
[   38.645963] ---[ end trace 0000000000000000 ]---
[   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
[   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
[   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
[   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
[   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
[   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
[   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
[   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
[   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
[   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
[   38.645963] PKRU: 55555554
[   38.645963] Kernel panic - not syncing: Fatal exception
[   38.645963] Kernel Offset: 0x1fc00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)

Fixes: ccb88e9549e7 ("crypto: ccp - Fix null pointer dereference in __sev_platform_shutdown_locked")
Cc: stable@vger.kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Liam Merwick June 3, 2024, 11:48 p.m. UTC | #1
On 03/06/2024 16:12, Kim Phillips wrote:
> Another DEBUG_TEST_DRIVER_REMOVE induced splat found, this time
> in __sev_snp_shutdown_locked().
> 
> [   38.625613] ccp 0000:55:00.5: enabling device (0000 -> 0002)
> [   38.633022] ccp 0000:55:00.5: sev enabled
> [   38.637498] ccp 0000:55:00.5: psp enabled
> [   38.642011] BUG: kernel NULL pointer dereference, address: 00000000000000f0
> [   38.645963] #PF: supervisor read access in kernel mode
> [   38.645963] #PF: error_code(0x0000) - not-present page
> [   38.645963] PGD 0 P4D 0
> [   38.645963] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC NOPTI
> [   38.645963] CPU: 262 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc1+ #29
> [   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
> [   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
> [   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
> [   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
> [   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
> [   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
> [   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
> [   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
> [   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
> [   38.645963] PKRU: 55555554
> [   38.645963] Call Trace:
> [   38.645963]  <TASK>
> [   38.645963]  ? __die_body+0x6f/0xb0
> [   38.645963]  ? __die+0xcc/0xf0
> [   38.645963]  ? page_fault_oops+0x330/0x3a0
> [   38.645963]  ? save_trace+0x2a5/0x360
> [   38.645963]  ? do_user_addr_fault+0x583/0x630
> [   38.645963]  ? exc_page_fault+0x81/0x120
> [   38.645963]  ? asm_exc_page_fault+0x2b/0x30
> [   38.645963]  ? __sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963]  __sev_firmware_shutdown+0x349/0x5b0
> [   38.645963]  ? pm_runtime_barrier+0x66/0xe0
> [   38.645963]  sev_dev_destroy+0x34/0xb0
> [   38.645963]  psp_dev_destroy+0x27/0x60
> [   38.645963]  sp_destroy+0x39/0x90
> [   38.645963]  sp_pci_remove+0x22/0x60
> [   38.645963]  pci_device_remove+0x4e/0x110
> [   38.645963]  really_probe+0x271/0x4e0
> [   38.645963]  __driver_probe_device+0x8f/0x160
> [   38.645963]  driver_probe_device+0x24/0x120
> [   38.645963]  __driver_attach+0xc7/0x280
> [   38.645963]  ? driver_attach+0x30/0x30
> [   38.645963]  bus_for_each_dev+0x10d/0x130
> [   38.645963]  driver_attach+0x22/0x30
> [   38.645963]  bus_add_driver+0x171/0x2b0
> [   38.645963]  ? unaccepted_memory_init_kdump+0x20/0x20
> [   38.645963]  driver_register+0x67/0x100
> [   38.645963]  __pci_register_driver+0x83/0x90
> [   38.645963]  sp_pci_init+0x22/0x30
> [   38.645963]  sp_mod_init+0x13/0x30
> [   38.645963]  do_one_initcall+0xb8/0x290
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
> [   38.645963]  ? local_clock+0x1c/0x60
> [   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? __lock_acquire+0xd90/0xe30
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? __create_object+0x66/0x100
> [   38.645963]  ? local_clock+0x1c/0x60
> [   38.645963]  ? __create_object+0x66/0x100
> [   38.645963]  ? parameq+0x1b/0x90
> [   38.645963]  ? parse_one+0x6d/0x1d0
> [   38.645963]  ? parse_args+0xd7/0x1f0
> [   38.645963]  ? do_initcall_level+0x180/0x180
> [   38.645963]  do_initcall_level+0xb0/0x180
> [   38.645963]  do_initcalls+0x60/0xa0
> [   38.645963]  ? kernel_init+0x1f/0x1d0
> [   38.645963]  do_basic_setup+0x41/0x50
> [   38.645963]  kernel_init_freeable+0x1ac/0x230
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  kernel_init+0x1f/0x1d0
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  ret_from_fork+0x3d/0x50
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  ret_from_fork_asm+0x11/0x20
> [   38.645963]  </TASK>
> [   38.645963] Modules linked in:
> [   38.645963] CR2: 00000000000000f0
> [   38.645963] ---[ end trace 0000000000000000 ]---
> [   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
> [   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
> [   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
> [   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
> [   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
> [   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
> [   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
> [   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
> [   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
> [   38.645963] PKRU: 55555554
> [   38.645963] Kernel panic - not syncing: Fatal exception
> [   38.645963] Kernel Offset: 0x1fc00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> 
> Fixes: ccb88e9549e7 ("crypto: ccp - Fix null pointer dereference in __sev_platform_shutdown_locked")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>


> ---
>   drivers/crypto/ccp/sev-dev.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 2102377f727b..1912bee22dd4 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1642,10 +1642,16 @@ static int sev_update_firmware(struct device *dev)
>   
>   static int __sev_snp_shutdown_locked(int *error, bool panic)
>   {
> -	struct sev_device *sev = psp_master->sev_data;
> +	struct psp_device *psp = psp_master;
> +	struct sev_device *sev;
>   	struct sev_data_snp_shutdown_ex data;
>   	int ret;
>   
> +	if (!psp || !psp->sev_data)
> +		return 0;
> +
> +	sev = psp->sev_data;
> +
>   	if (!sev->snp_initialized)
>   		return 0;
>
Mario Limonciello June 4, 2024, 1:22 a.m. UTC | #2
On 6/3/2024 10:12 AM, Kim Phillips wrote:
> Another DEBUG_TEST_DRIVER_REMOVE induced splat found, this time
> in __sev_snp_shutdown_locked().
> 
> [   38.625613] ccp 0000:55:00.5: enabling device (0000 -> 0002)
> [   38.633022] ccp 0000:55:00.5: sev enabled
> [   38.637498] ccp 0000:55:00.5: psp enabled
> [   38.642011] BUG: kernel NULL pointer dereference, address: 00000000000000f0
> [   38.645963] #PF: supervisor read access in kernel mode
> [   38.645963] #PF: error_code(0x0000) - not-present page
> [   38.645963] PGD 0 P4D 0
> [   38.645963] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC NOPTI
> [   38.645963] CPU: 262 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc1+ #29
> [   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
> [   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
> [   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
> [   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
> [   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
> [   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
> [   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
> [   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
> [   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
> [   38.645963] PKRU: 55555554
> [   38.645963] Call Trace:
> [   38.645963]  <TASK>
> [   38.645963]  ? __die_body+0x6f/0xb0
> [   38.645963]  ? __die+0xcc/0xf0
> [   38.645963]  ? page_fault_oops+0x330/0x3a0
> [   38.645963]  ? save_trace+0x2a5/0x360
> [   38.645963]  ? do_user_addr_fault+0x583/0x630
> [   38.645963]  ? exc_page_fault+0x81/0x120
> [   38.645963]  ? asm_exc_page_fault+0x2b/0x30
> [   38.645963]  ? __sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963]  __sev_firmware_shutdown+0x349/0x5b0
> [   38.645963]  ? pm_runtime_barrier+0x66/0xe0
> [   38.645963]  sev_dev_destroy+0x34/0xb0
> [   38.645963]  psp_dev_destroy+0x27/0x60
> [   38.645963]  sp_destroy+0x39/0x90
> [   38.645963]  sp_pci_remove+0x22/0x60
> [   38.645963]  pci_device_remove+0x4e/0x110
> [   38.645963]  really_probe+0x271/0x4e0
> [   38.645963]  __driver_probe_device+0x8f/0x160
> [   38.645963]  driver_probe_device+0x24/0x120
> [   38.645963]  __driver_attach+0xc7/0x280
> [   38.645963]  ? driver_attach+0x30/0x30
> [   38.645963]  bus_for_each_dev+0x10d/0x130
> [   38.645963]  driver_attach+0x22/0x30
> [   38.645963]  bus_add_driver+0x171/0x2b0
> [   38.645963]  ? unaccepted_memory_init_kdump+0x20/0x20
> [   38.645963]  driver_register+0x67/0x100
> [   38.645963]  __pci_register_driver+0x83/0x90
> [   38.645963]  sp_pci_init+0x22/0x30
> [   38.645963]  sp_mod_init+0x13/0x30
> [   38.645963]  do_one_initcall+0xb8/0x290
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
> [   38.645963]  ? local_clock+0x1c/0x60
> [   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? __lock_acquire+0xd90/0xe30
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? __create_object+0x66/0x100
> [   38.645963]  ? local_clock+0x1c/0x60
> [   38.645963]  ? __create_object+0x66/0x100
> [   38.645963]  ? parameq+0x1b/0x90
> [   38.645963]  ? parse_one+0x6d/0x1d0
> [   38.645963]  ? parse_args+0xd7/0x1f0
> [   38.645963]  ? do_initcall_level+0x180/0x180
> [   38.645963]  do_initcall_level+0xb0/0x180
> [   38.645963]  do_initcalls+0x60/0xa0
> [   38.645963]  ? kernel_init+0x1f/0x1d0
> [   38.645963]  do_basic_setup+0x41/0x50
> [   38.645963]  kernel_init_freeable+0x1ac/0x230
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  kernel_init+0x1f/0x1d0
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  ret_from_fork+0x3d/0x50
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  ret_from_fork_asm+0x11/0x20
> [   38.645963]  </TASK>
> [   38.645963] Modules linked in:
> [   38.645963] CR2: 00000000000000f0
> [   38.645963] ---[ end trace 0000000000000000 ]---
> [   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
> [   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
> [   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
> [   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
> [   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
> [   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
> [   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
> [   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
> [   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
> [   38.645963] PKRU: 55555554
> [   38.645963] Kernel panic - not syncing: Fatal exception
> [   38.645963] Kernel Offset: 0x1fc00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> 
> Fixes: ccb88e9549e7 ("crypto: ccp - Fix null pointer dereference in __sev_platform_shutdown_locked")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

> ---
>   drivers/crypto/ccp/sev-dev.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 2102377f727b..1912bee22dd4 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1642,10 +1642,16 @@ static int sev_update_firmware(struct device *dev)
>   
>   static int __sev_snp_shutdown_locked(int *error, bool panic)
>   {
> -	struct sev_device *sev = psp_master->sev_data;
> +	struct psp_device *psp = psp_master;
> +	struct sev_device *sev;
>   	struct sev_data_snp_shutdown_ex data;
>   	int ret;
>   
> +	if (!psp || !psp->sev_data)
> +		return 0;
> +
> +	sev = psp->sev_data;
> +
>   	if (!sev->snp_initialized)
>   		return 0;
>
John Allen June 4, 2024, 4:24 p.m. UTC | #3
On Mon, Jun 03, 2024 at 10:12:12AM -0500, Kim Phillips wrote:
> Another DEBUG_TEST_DRIVER_REMOVE induced splat found, this time
> in __sev_snp_shutdown_locked().
> 
> [   38.625613] ccp 0000:55:00.5: enabling device (0000 -> 0002)
> [   38.633022] ccp 0000:55:00.5: sev enabled
> [   38.637498] ccp 0000:55:00.5: psp enabled
> [   38.642011] BUG: kernel NULL pointer dereference, address: 00000000000000f0
> [   38.645963] #PF: supervisor read access in kernel mode
> [   38.645963] #PF: error_code(0x0000) - not-present page
> [   38.645963] PGD 0 P4D 0
> [   38.645963] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC NOPTI
> [   38.645963] CPU: 262 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc1+ #29
> [   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
> [   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
> [   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
> [   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
> [   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
> [   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
> [   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
> [   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
> [   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
> [   38.645963] PKRU: 55555554
> [   38.645963] Call Trace:
> [   38.645963]  <TASK>
> [   38.645963]  ? __die_body+0x6f/0xb0
> [   38.645963]  ? __die+0xcc/0xf0
> [   38.645963]  ? page_fault_oops+0x330/0x3a0
> [   38.645963]  ? save_trace+0x2a5/0x360
> [   38.645963]  ? do_user_addr_fault+0x583/0x630
> [   38.645963]  ? exc_page_fault+0x81/0x120
> [   38.645963]  ? asm_exc_page_fault+0x2b/0x30
> [   38.645963]  ? __sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963]  __sev_firmware_shutdown+0x349/0x5b0
> [   38.645963]  ? pm_runtime_barrier+0x66/0xe0
> [   38.645963]  sev_dev_destroy+0x34/0xb0
> [   38.645963]  psp_dev_destroy+0x27/0x60
> [   38.645963]  sp_destroy+0x39/0x90
> [   38.645963]  sp_pci_remove+0x22/0x60
> [   38.645963]  pci_device_remove+0x4e/0x110
> [   38.645963]  really_probe+0x271/0x4e0
> [   38.645963]  __driver_probe_device+0x8f/0x160
> [   38.645963]  driver_probe_device+0x24/0x120
> [   38.645963]  __driver_attach+0xc7/0x280
> [   38.645963]  ? driver_attach+0x30/0x30
> [   38.645963]  bus_for_each_dev+0x10d/0x130
> [   38.645963]  driver_attach+0x22/0x30
> [   38.645963]  bus_add_driver+0x171/0x2b0
> [   38.645963]  ? unaccepted_memory_init_kdump+0x20/0x20
> [   38.645963]  driver_register+0x67/0x100
> [   38.645963]  __pci_register_driver+0x83/0x90
> [   38.645963]  sp_pci_init+0x22/0x30
> [   38.645963]  sp_mod_init+0x13/0x30
> [   38.645963]  do_one_initcall+0xb8/0x290
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
> [   38.645963]  ? local_clock+0x1c/0x60
> [   38.645963]  ? stack_depot_save_flags+0x21e/0x6a0
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? __lock_acquire+0xd90/0xe30
> [   38.645963]  ? sched_clock_noinstr+0xd/0x10
> [   38.645963]  ? local_clock_noinstr+0x3e/0x100
> [   38.645963]  ? __create_object+0x66/0x100
> [   38.645963]  ? local_clock+0x1c/0x60
> [   38.645963]  ? __create_object+0x66/0x100
> [   38.645963]  ? parameq+0x1b/0x90
> [   38.645963]  ? parse_one+0x6d/0x1d0
> [   38.645963]  ? parse_args+0xd7/0x1f0
> [   38.645963]  ? do_initcall_level+0x180/0x180
> [   38.645963]  do_initcall_level+0xb0/0x180
> [   38.645963]  do_initcalls+0x60/0xa0
> [   38.645963]  ? kernel_init+0x1f/0x1d0
> [   38.645963]  do_basic_setup+0x41/0x50
> [   38.645963]  kernel_init_freeable+0x1ac/0x230
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  kernel_init+0x1f/0x1d0
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  ret_from_fork+0x3d/0x50
> [   38.645963]  ? rest_init+0x1f0/0x1f0
> [   38.645963]  ret_from_fork_asm+0x11/0x20
> [   38.645963]  </TASK>
> [   38.645963] Modules linked in:
> [   38.645963] CR2: 00000000000000f0
> [   38.645963] ---[ end trace 0000000000000000 ]---
> [   38.645963] RIP: 0010:__sev_snp_shutdown_locked+0x2e/0x150
> [   38.645963] Code: 00 55 48 89 e5 41 57 41 56 41 54 53 48 83 ec 10 41 89 f7 49 89 fe 65 48 8b 04 25 28 00 00 00 48 89 45 d8 48 8b 05 6a 5a 7f 06 <4c> 8b a0 f0 00 00 00 41 0f b6 9c 24 a2 00 00 00 48 83 fb 02 0f 83
> [   38.645963] RSP: 0018:ffffb2ea4014b7b8 EFLAGS: 00010286
> [   38.645963] RAX: 0000000000000000 RBX: ffff9e4acd2e0a28 RCX: 0000000000000000
> [   38.645963] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffb2ea4014b808
> [   38.645963] RBP: ffffb2ea4014b7e8 R08: 0000000000000106 R09: 000000000003d9c0
> [   38.645963] R10: 0000000000000001 R11: ffffffffa39ff070 R12: ffff9e49d40590c8
> [   38.645963] R13: 0000000000000000 R14: ffffb2ea4014b808 R15: 0000000000000000
> [   38.645963] FS:  0000000000000000(0000) GS:ffff9e58b1e00000(0000) knlGS:0000000000000000
> [   38.645963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   38.645963] CR2: 00000000000000f0 CR3: 0000000418a3e001 CR4: 0000000000770ef0
> [   38.645963] PKRU: 55555554
> [   38.645963] Kernel panic - not syncing: Fatal exception
> [   38.645963] Kernel Offset: 0x1fc00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> 
> Fixes: ccb88e9549e7 ("crypto: ccp - Fix null pointer dereference in __sev_platform_shutdown_locked")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>

I just have a couple of nits on the commit description. Please trim the
"[   38.645963]" from the beginning of the kernel trace. Also, a
description of the problem and the fix would be nice. Maybe something
along the lines of:

"Fix a null pointer dereference induced with DEBUG_TEST_DRIVER_REMOVE.
Return from __sev_snp_shutdown_locked if the sev_device or the
psp_device structs are not initialized. Without the fix, the driver will
produce the following splat:
..."

Otherwise, patch content looks good to me.

Reviewed-by: John Allen <john.allen@amd.com>

> ---
>  drivers/crypto/ccp/sev-dev.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 2102377f727b..1912bee22dd4 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1642,10 +1642,16 @@ static int sev_update_firmware(struct device *dev)
>  
>  static int __sev_snp_shutdown_locked(int *error, bool panic)
>  {
> -	struct sev_device *sev = psp_master->sev_data;
> +	struct psp_device *psp = psp_master;
> +	struct sev_device *sev;
>  	struct sev_data_snp_shutdown_ex data;
>  	int ret;
>  
> +	if (!psp || !psp->sev_data)
> +		return 0;
> +
> +	sev = psp->sev_data;
> +
>  	if (!sev->snp_initialized)
>  		return 0;
>  
> -- 
> 2.34.1
>
Tom Lendacky June 4, 2024, 4:28 p.m. UTC | #4
On 6/3/24 10:12, Kim Phillips wrote:
> Another DEBUG_TEST_DRIVER_REMOVE induced splat found, this time
> in __sev_snp_shutdown_locked().
> 

> 
> Fixes: ccb88e9549e7 ("crypto: ccp - Fix null pointer dereference in __sev_platform_shutdown_locked")

Shouldn't the Fixes tag be:

1ca5614b84ee ("crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP")

Thanks,
Tom

> Cc: stable@vger.kernel.org
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
> ---
>   drivers/crypto/ccp/sev-dev.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
diff mbox series

Patch

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 2102377f727b..1912bee22dd4 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1642,10 +1642,16 @@  static int sev_update_firmware(struct device *dev)
 
 static int __sev_snp_shutdown_locked(int *error, bool panic)
 {
-	struct sev_device *sev = psp_master->sev_data;
+	struct psp_device *psp = psp_master;
+	struct sev_device *sev;
 	struct sev_data_snp_shutdown_ex data;
 	int ret;
 
+	if (!psp || !psp->sev_data)
+		return 0;
+
+	sev = psp->sev_data;
+
 	if (!sev->snp_initialized)
 		return 0;