Message ID | 20240125231253.3122579-1-kim.phillips@amd.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Herbert Xu |
Headers | show |
Series | crypto: ccp - Fix null pointer dereference in __sev_platform_shutdown_locked | expand |
On 25/01/2024 23:12, Kim Phillips wrote: > The SEV platform device can be shutdown with a null psp_master, > e.g., using DEBUG_TEST_DRIVER_REMOVE. Found using KASAN: > > [ 137.148210] ccp 0000:23:00.1: enabling device (0000 -> 0002) > [ 137.162647] ccp 0000:23:00.1: no command queues available > [ 137.170598] ccp 0000:23:00.1: sev enabled > [ 137.174645] ccp 0000:23:00.1: psp enabled > [ 137.178890] general protection fault, probably for non-canonical address 0xdffffc000000001e: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN NOPTI > [ 137.182693] KASAN: null-ptr-deref in range [0x00000000000000f0-0x00000000000000f7] > [ 137.182693] CPU: 93 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc1+ #311 > [ 137.182693] RIP: 0010:__sev_platform_shutdown_locked+0x51/0x180 > [ 137.182693] Code: 08 80 3c 08 00 0f 85 0e 01 00 00 48 8b 1d 67 b6 01 08 48 b8 00 00 00 00 00 fc ff df 48 8d bb f0 00 00 00 48 89 f9 48 c1 e9 03 <80> 3c 01 00 0f 85 fe 00 00 00 48 8b 9b f0 00 00 00 48 85 db 74 2c > [ 137.182693] RSP: 0018:ffffc900000cf9b0 EFLAGS: 00010216 > [ 137.182693] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 000000000000001e > [ 137.182693] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 00000000000000f0 > [ 137.182693] RBP: ffffc900000cf9c8 R08: 0000000000000000 R09: fffffbfff58f5a66 > [ 137.182693] R10: ffffc900000cf9c8 R11: ffffffffac7ad32f R12: ffff8881e5052c28 > [ 137.182693] R13: ffff8881e5052c28 R14: ffff8881758e43e8 R15: ffffffffac64abf8 > [ 137.182693] FS: 0000000000000000(0000) GS:ffff889de7000000(0000) knlGS:0000000000000000 > [ 137.182693] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 137.182693] CR2: 0000000000000000 CR3: 0000001cf7c7e000 CR4: 0000000000350ef0 > [ 137.182693] Call Trace: > [ 137.182693] <TASK> > [ 137.182693] ? show_regs+0x6c/0x80 > [ 137.182693] ? __die_body+0x24/0x70 > [ 137.182693] ? die_addr+0x4b/0x80 > [ 137.182693] ? exc_general_protection+0x126/0x230 > [ 137.182693] ? asm_exc_general_protection+0x2b/0x30 > [ 137.182693] ? __sev_platform_shutdown_locked+0x51/0x180 > [ 137.182693] sev_firmware_shutdown.isra.0+0x1e/0x80 > [ 137.182693] sev_dev_destroy+0x49/0x100 > [ 137.182693] psp_dev_destroy+0x47/0xb0 > [ 137.182693] sp_destroy+0xbb/0x240 > [ 137.182693] sp_pci_remove+0x45/0x60 > [ 137.182693] pci_device_remove+0xaa/0x1d0 > [ 137.182693] device_remove+0xc7/0x170 > [ 137.182693] really_probe+0x374/0xbe0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] __driver_probe_device+0x199/0x460 > [ 137.182693] driver_probe_device+0x4e/0xd0 > [ 137.182693] __driver_attach+0x191/0x3d0 > [ 137.182693] ? __pfx___driver_attach+0x10/0x10 > [ 137.182693] bus_for_each_dev+0x100/0x190 > [ 137.182693] ? __pfx_bus_for_each_dev+0x10/0x10 > [ 137.182693] ? __kasan_check_read+0x15/0x20 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? _raw_spin_unlock+0x27/0x50 > [ 137.182693] driver_attach+0x41/0x60 > [ 137.182693] bus_add_driver+0x2a8/0x580 > [ 137.182693] driver_register+0x141/0x480 > [ 137.182693] __pci_register_driver+0x1d6/0x2a0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? esrt_sysfs_init+0x1cd/0x5d0 > [ 137.182693] ? __pfx_sp_mod_init+0x10/0x10 > [ 137.182693] sp_pci_init+0x22/0x30 > [ 137.182693] sp_mod_init+0x14/0x30 > [ 137.182693] ? __pfx_sp_mod_init+0x10/0x10 > [ 137.182693] do_one_initcall+0xd1/0x470 > [ 137.182693] ? __pfx_do_one_initcall+0x10/0x10 > [ 137.182693] ? parameq+0x80/0xf0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? __kmalloc+0x3b0/0x4e0 > [ 137.182693] ? kernel_init_freeable+0x92d/0x1050 > [ 137.182693] ? kasan_populate_vmalloc_pte+0x171/0x190 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] kernel_init_freeable+0xa64/0x1050 > [ 137.182693] ? __pfx_kernel_init+0x10/0x10 > [ 137.182693] kernel_init+0x24/0x160 > [ 137.182693] ? __switch_to_asm+0x3e/0x70 > [ 137.182693] ret_from_fork+0x40/0x80 > [ 137.182693] ? __pfx_kernel_init+0x10/0x10 > [ 137.182693] ret_from_fork_asm+0x1b/0x30 > [ 137.182693] </TASK> > [ 137.182693] Modules linked in: > [ 137.538483] ---[ end trace 0000000000000000 ]--- > > Fixes: 1b05ece0c9315 ("crypto: ccp - During shutdown, check SEV data pointer before using") checkpatch warns about SHA1 of Fixes: having more than 12 chars... However, although 1b05ece0c931 is the last commit to change this functionality, I think this issue exists prior to that. 5441a07a127f ("crypto: ccp - shutdown SEV firmware on kexec") might be more appropriate so that it'd get applied to linux-5.15.y (where 1b05ece0c931 and 5441a07a127f have been backported to also) This patch applies cleanly to linux-5.15.y. > Cc: stable@vger.kernel.org > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Kim Phillips <kim.phillips@amd.com> Code changes LGTM, so Reviewed-by: Liam Merwick <liam.merwick@oracle.com> > --- > drivers/crypto/ccp/sev-dev.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index fcaccd0b5a65..53b217a62104 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -534,10 +534,16 @@ EXPORT_SYMBOL_GPL(sev_platform_init); > > static int __sev_platform_shutdown_locked(int *error) > { > - struct sev_device *sev = psp_master->sev_data; > + struct psp_device *psp = psp_master; > + struct sev_device *sev; > int ret; > > - if (!sev || sev->state == SEV_STATE_UNINIT) > + if (!psp || !psp->sev_data) > + return 0; > + > + sev = psp->sev_data; > + > + if (sev->state == SEV_STATE_UNINIT) > return 0; > > ret = __sev_do_cmd_locked(SEV_CMD_SHUTDOWN, NULL, error);
On 1/26/24 5:59 AM, Liam Merwick wrote: > On 25/01/2024 23:12, Kim Phillips wrote: >> Fixes: 1b05ece0c9315 ("crypto: ccp - During shutdown, check SEV data pointer before using") > > checkpatch warns about SHA1 of Fixes: having more than 12 chars... > > However, although 1b05ece0c931 is the last commit to change this > functionality, I think this issue exists prior to that. > > 5441a07a127f ("crypto: ccp - shutdown SEV firmware on kexec") > might be more appropriate so that it'd get applied to linux-5.15.y > (where 1b05ece0c931 and 5441a07a127f have been backported to also) > This patch applies cleanly to linux-5.15.y. 1b05ece0c931 fixes 5441a07a127f, and I'm assuming Fixes: are transitive. >> Cc: stable@vger.kernel.org >> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Kim Phillips <kim.phillips@amd.com> > > Code changes LGTM, so > Reviewed-by: Liam Merwick <liam.merwick@oracle.com> Thanks for your review! Kim
On Thu, Jan 25, 2024 at 05:12:53PM -0600, Kim Phillips wrote: > The SEV platform device can be shutdown with a null psp_master, > e.g., using DEBUG_TEST_DRIVER_REMOVE. Found using KASAN: > > [ 137.148210] ccp 0000:23:00.1: enabling device (0000 -> 0002) > [ 137.162647] ccp 0000:23:00.1: no command queues available > [ 137.170598] ccp 0000:23:00.1: sev enabled > [ 137.174645] ccp 0000:23:00.1: psp enabled > [ 137.178890] general protection fault, probably for non-canonical address 0xdffffc000000001e: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN NOPTI > [ 137.182693] KASAN: null-ptr-deref in range [0x00000000000000f0-0x00000000000000f7] > [ 137.182693] CPU: 93 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc1+ #311 > [ 137.182693] RIP: 0010:__sev_platform_shutdown_locked+0x51/0x180 > [ 137.182693] Code: 08 80 3c 08 00 0f 85 0e 01 00 00 48 8b 1d 67 b6 01 08 48 b8 00 00 00 00 00 fc ff df 48 8d bb f0 00 00 00 48 89 f9 48 c1 e9 03 <80> 3c 01 00 0f 85 fe 00 00 00 48 8b 9b f0 00 00 00 48 85 db 74 2c > [ 137.182693] RSP: 0018:ffffc900000cf9b0 EFLAGS: 00010216 > [ 137.182693] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 000000000000001e > [ 137.182693] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 00000000000000f0 > [ 137.182693] RBP: ffffc900000cf9c8 R08: 0000000000000000 R09: fffffbfff58f5a66 > [ 137.182693] R10: ffffc900000cf9c8 R11: ffffffffac7ad32f R12: ffff8881e5052c28 > [ 137.182693] R13: ffff8881e5052c28 R14: ffff8881758e43e8 R15: ffffffffac64abf8 > [ 137.182693] FS: 0000000000000000(0000) GS:ffff889de7000000(0000) knlGS:0000000000000000 > [ 137.182693] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 137.182693] CR2: 0000000000000000 CR3: 0000001cf7c7e000 CR4: 0000000000350ef0 > [ 137.182693] Call Trace: > [ 137.182693] <TASK> > [ 137.182693] ? show_regs+0x6c/0x80 > [ 137.182693] ? __die_body+0x24/0x70 > [ 137.182693] ? die_addr+0x4b/0x80 > [ 137.182693] ? exc_general_protection+0x126/0x230 > [ 137.182693] ? asm_exc_general_protection+0x2b/0x30 > [ 137.182693] ? __sev_platform_shutdown_locked+0x51/0x180 > [ 137.182693] sev_firmware_shutdown.isra.0+0x1e/0x80 > [ 137.182693] sev_dev_destroy+0x49/0x100 > [ 137.182693] psp_dev_destroy+0x47/0xb0 > [ 137.182693] sp_destroy+0xbb/0x240 > [ 137.182693] sp_pci_remove+0x45/0x60 > [ 137.182693] pci_device_remove+0xaa/0x1d0 > [ 137.182693] device_remove+0xc7/0x170 > [ 137.182693] really_probe+0x374/0xbe0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] __driver_probe_device+0x199/0x460 > [ 137.182693] driver_probe_device+0x4e/0xd0 > [ 137.182693] __driver_attach+0x191/0x3d0 > [ 137.182693] ? __pfx___driver_attach+0x10/0x10 > [ 137.182693] bus_for_each_dev+0x100/0x190 > [ 137.182693] ? __pfx_bus_for_each_dev+0x10/0x10 > [ 137.182693] ? __kasan_check_read+0x15/0x20 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? _raw_spin_unlock+0x27/0x50 > [ 137.182693] driver_attach+0x41/0x60 > [ 137.182693] bus_add_driver+0x2a8/0x580 > [ 137.182693] driver_register+0x141/0x480 > [ 137.182693] __pci_register_driver+0x1d6/0x2a0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? esrt_sysfs_init+0x1cd/0x5d0 > [ 137.182693] ? __pfx_sp_mod_init+0x10/0x10 > [ 137.182693] sp_pci_init+0x22/0x30 > [ 137.182693] sp_mod_init+0x14/0x30 > [ 137.182693] ? __pfx_sp_mod_init+0x10/0x10 > [ 137.182693] do_one_initcall+0xd1/0x470 > [ 137.182693] ? __pfx_do_one_initcall+0x10/0x10 > [ 137.182693] ? parameq+0x80/0xf0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? __kmalloc+0x3b0/0x4e0 > [ 137.182693] ? kernel_init_freeable+0x92d/0x1050 > [ 137.182693] ? kasan_populate_vmalloc_pte+0x171/0x190 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] kernel_init_freeable+0xa64/0x1050 > [ 137.182693] ? __pfx_kernel_init+0x10/0x10 > [ 137.182693] kernel_init+0x24/0x160 > [ 137.182693] ? __switch_to_asm+0x3e/0x70 > [ 137.182693] ret_from_fork+0x40/0x80 > [ 137.182693] ? __pfx_kernel_init+0x10/0x10 > [ 137.182693] ret_from_fork_asm+0x1b/0x30 > [ 137.182693] </TASK> > [ 137.182693] Modules linked in: > [ 137.538483] ---[ end trace 0000000000000000 ]--- > > Fixes: 1b05ece0c9315 ("crypto: ccp - During shutdown, check SEV data pointer before using") > Cc: stable@vger.kernel.org > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Kim Phillips <kim.phillips@amd.com> Acked-by: John Allen <john.allen@amd.com>
On Thu, Jan 25, 2024 at 05:12:53PM -0600, Kim Phillips wrote: > The SEV platform device can be shutdown with a null psp_master, > e.g., using DEBUG_TEST_DRIVER_REMOVE. Found using KASAN: > > [ 137.148210] ccp 0000:23:00.1: enabling device (0000 -> 0002) > [ 137.162647] ccp 0000:23:00.1: no command queues available > [ 137.170598] ccp 0000:23:00.1: sev enabled > [ 137.174645] ccp 0000:23:00.1: psp enabled > [ 137.178890] general protection fault, probably for non-canonical address 0xdffffc000000001e: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN NOPTI > [ 137.182693] KASAN: null-ptr-deref in range [0x00000000000000f0-0x00000000000000f7] > [ 137.182693] CPU: 93 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc1+ #311 > [ 137.182693] RIP: 0010:__sev_platform_shutdown_locked+0x51/0x180 > [ 137.182693] Code: 08 80 3c 08 00 0f 85 0e 01 00 00 48 8b 1d 67 b6 01 08 48 b8 00 00 00 00 00 fc ff df 48 8d bb f0 00 00 00 48 89 f9 48 c1 e9 03 <80> 3c 01 00 0f 85 fe 00 00 00 48 8b 9b f0 00 00 00 48 85 db 74 2c > [ 137.182693] RSP: 0018:ffffc900000cf9b0 EFLAGS: 00010216 > [ 137.182693] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 000000000000001e > [ 137.182693] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 00000000000000f0 > [ 137.182693] RBP: ffffc900000cf9c8 R08: 0000000000000000 R09: fffffbfff58f5a66 > [ 137.182693] R10: ffffc900000cf9c8 R11: ffffffffac7ad32f R12: ffff8881e5052c28 > [ 137.182693] R13: ffff8881e5052c28 R14: ffff8881758e43e8 R15: ffffffffac64abf8 > [ 137.182693] FS: 0000000000000000(0000) GS:ffff889de7000000(0000) knlGS:0000000000000000 > [ 137.182693] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 137.182693] CR2: 0000000000000000 CR3: 0000001cf7c7e000 CR4: 0000000000350ef0 > [ 137.182693] Call Trace: > [ 137.182693] <TASK> > [ 137.182693] ? show_regs+0x6c/0x80 > [ 137.182693] ? __die_body+0x24/0x70 > [ 137.182693] ? die_addr+0x4b/0x80 > [ 137.182693] ? exc_general_protection+0x126/0x230 > [ 137.182693] ? asm_exc_general_protection+0x2b/0x30 > [ 137.182693] ? __sev_platform_shutdown_locked+0x51/0x180 > [ 137.182693] sev_firmware_shutdown.isra.0+0x1e/0x80 > [ 137.182693] sev_dev_destroy+0x49/0x100 > [ 137.182693] psp_dev_destroy+0x47/0xb0 > [ 137.182693] sp_destroy+0xbb/0x240 > [ 137.182693] sp_pci_remove+0x45/0x60 > [ 137.182693] pci_device_remove+0xaa/0x1d0 > [ 137.182693] device_remove+0xc7/0x170 > [ 137.182693] really_probe+0x374/0xbe0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] __driver_probe_device+0x199/0x460 > [ 137.182693] driver_probe_device+0x4e/0xd0 > [ 137.182693] __driver_attach+0x191/0x3d0 > [ 137.182693] ? __pfx___driver_attach+0x10/0x10 > [ 137.182693] bus_for_each_dev+0x100/0x190 > [ 137.182693] ? __pfx_bus_for_each_dev+0x10/0x10 > [ 137.182693] ? __kasan_check_read+0x15/0x20 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? _raw_spin_unlock+0x27/0x50 > [ 137.182693] driver_attach+0x41/0x60 > [ 137.182693] bus_add_driver+0x2a8/0x580 > [ 137.182693] driver_register+0x141/0x480 > [ 137.182693] __pci_register_driver+0x1d6/0x2a0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? esrt_sysfs_init+0x1cd/0x5d0 > [ 137.182693] ? __pfx_sp_mod_init+0x10/0x10 > [ 137.182693] sp_pci_init+0x22/0x30 > [ 137.182693] sp_mod_init+0x14/0x30 > [ 137.182693] ? __pfx_sp_mod_init+0x10/0x10 > [ 137.182693] do_one_initcall+0xd1/0x470 > [ 137.182693] ? __pfx_do_one_initcall+0x10/0x10 > [ 137.182693] ? parameq+0x80/0xf0 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] ? __kmalloc+0x3b0/0x4e0 > [ 137.182693] ? kernel_init_freeable+0x92d/0x1050 > [ 137.182693] ? kasan_populate_vmalloc_pte+0x171/0x190 > [ 137.182693] ? srso_return_thunk+0x5/0x5f > [ 137.182693] kernel_init_freeable+0xa64/0x1050 > [ 137.182693] ? __pfx_kernel_init+0x10/0x10 > [ 137.182693] kernel_init+0x24/0x160 > [ 137.182693] ? __switch_to_asm+0x3e/0x70 > [ 137.182693] ret_from_fork+0x40/0x80 > [ 137.182693] ? __pfx_kernel_init+0x10/0x10 > [ 137.182693] ret_from_fork_asm+0x1b/0x30 > [ 137.182693] </TASK> > [ 137.182693] Modules linked in: > [ 137.538483] ---[ end trace 0000000000000000 ]--- > > Fixes: 1b05ece0c9315 ("crypto: ccp - During shutdown, check SEV data pointer before using") > Cc: stable@vger.kernel.org > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Kim Phillips <kim.phillips@amd.com> > --- > drivers/crypto/ccp/sev-dev.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) Patch applied. Thanks.
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index fcaccd0b5a65..53b217a62104 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -534,10 +534,16 @@ EXPORT_SYMBOL_GPL(sev_platform_init); static int __sev_platform_shutdown_locked(int *error) { - struct sev_device *sev = psp_master->sev_data; + struct psp_device *psp = psp_master; + struct sev_device *sev; int ret; - if (!sev || sev->state == SEV_STATE_UNINIT) + if (!psp || !psp->sev_data) + return 0; + + sev = psp->sev_data; + + if (sev->state == SEV_STATE_UNINIT) return 0; ret = __sev_do_cmd_locked(SEV_CMD_SHUTDOWN, NULL, error);