* [PATCH 0/2] crypto: qat: fix couple crashes duing error handling
@ 2021-03-12 7:43 Tong Zhang
2021-03-12 7:43 ` [PATCH 1/2] crypto: qat - dont release uninitialized resources Tong Zhang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tong Zhang @ 2021-03-12 7:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Wojciech Ziemba,
Andy Shevchenko, Fiona Trahe, Tong Zhang, qat-linux,
linux-crypto, linux-kernel
There are a couple of issues in qat error handling. Those drivers tries to
release resources that is not initialized. This patch series tries to fix
crashes caused by incorrect error handling.
Tong Zhang (2):
crypto: qat - dont release uninitialized resources
crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init
drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++--
drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++--
drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++----
drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++--
4 files changed, 19 insertions(+), 10 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] crypto: qat - dont release uninitialized resources 2021-03-12 7:43 [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang @ 2021-03-12 7:43 ` Tong Zhang 2021-03-12 7:43 ` [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-12 11:32 ` [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Andy Shevchenko 2 siblings, 0 replies; 6+ messages in thread From: Tong Zhang @ 2021-03-12 7:43 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Wojciech Ziemba, Andy Shevchenko, Fiona Trahe, Tong Zhang, qat-linux, linux-crypto, linux-kernel adf_vf_isr_resource_alloc() is not unwinding correctly when error happens and it trys to release uninitialized resources. To fix this, only release initialized resources. [ 1.792594] ------------[ cut here ]------------ [ 1.792845] Trying to free already-free IRQ 11 [ 1.793091] WARNING: CPU: 0 PID: 182 at kernel/irq/manage.c:1821 free_irq+0x202/0x380 [ 1.793518] Modules linked in: qat_c3xxxvf(+) drm snd_pcm intel_qat snd_timer snd joydev psmouse 4 [ 1.794998] CPU: 0 PID: 182 Comm: systemd-udevd Not tainted 5.12.0-rc2+ #78 [ 1.795379] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-4 [ 1.795986] RIP: 0010:free_irq+0x202/0x380 [ 1.796211] Code: e8 23 6a 1b 00 48 83 c4 10 4c 89 e0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 8b 75 d0 40 [ 1.797215] RSP: 0018:ffffc90000ce3970 EFLAGS: 00010082 [ 1.797500] RAX: 0000000000000000 RBX: ffff888104b75200 RCX: 0000000000000027 [ 1.797886] RDX: 0000000000000027 RSI: 00000000ffffdfff RDI: ffff88817bc18448 [ 1.798270] RBP: ffffc90000ce39a8 R08: ffff88817bc18440 R09: ffffc90000ce3730 [ 1.798655] R10: 0000000000000001 R11: 0000000000000001 R12: ffff8881001c2200 [ 1.799040] R13: ffff8881001c2360 R14: ffff8881001c22a4 R15: ffff8881008b30c8 [ 1.799434] FS: 00007f2313cd38c0(0000) GS:ffff88817bc00000(0000) knlGS:0000000000000000 [ 1.799871] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1.800183] CR2: 00007ffc6f4b4080 CR3: 0000000104a3c000 CR4: 00000000000006f0 [ 1.800569] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1.800954] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 1.801340] Call Trace: [ 1.801477] adf_vf_isr_resource_free+0x32/0xb0 [intel_qat] [ 1.801785] adf_vf_isr_resource_alloc+0x14d/0x150 [intel_qat] [ 1.802105] adf_dev_init+0xba/0x140 [intel_qat] [ 1.802365] adf_probe+0x32f/0x370 [qat_c3xxxvf] [ 1.802620] local_pci_probe+0x48/0x80 [ 1.802827] pci_device_probe+0x10f/0x1c0 [ 1.803048] really_probe+0xfb/0x420 [ 1.803249] driver_probe_device+0xe9/0x160 [ 1.803479] device_driver_attach+0x5d/0x70 [ 1.803709] __driver_attach+0x8f/0x150 [ 1.803920] ? device_driver_attach+0x70/0x70 [ 1.804159] bus_for_each_dev+0x7e/0xc0 [ 1.804370] driver_attach+0x1e/0x20 [ 1.804568] bus_add_driver+0x152/0x1f0 [ 1.804779] driver_register+0x74/0xd0 [ 1.804986] ? 0xffffffffc009b000 [ 1.805169] __pci_register_driver+0x54/0x60 [ 1.805404] adfdrv_init+0x34/0x1000 [qat_c3xxxvf] [ 1.805667] do_one_initcall+0x48/0x1d0 [ 1.805879] ? __slab_alloc+0x20/0x40 [ 1.806081] ? kmem_cache_alloc_trace+0x390/0x440 [ 1.806338] ? do_init_module+0x28/0x250 [ 1.806555] do_init_module+0x62/0x250 [ 1.806762] load_module+0x23ee/0x26a0 [ 1.806970] __do_sys_finit_module+0xc2/0x120 [ 1.807211] ? __do_sys_finit_module+0xc2/0x120 [ 1.807460] __x64_sys_finit_module+0x1a/0x20 [ 1.807693] do_syscall_64+0x38/0x90 [ 1.807892] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 1.808167] RIP: 0033:0x7f2312b50469 [ 1.808365] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 [ 1.809368] RSP: 002b:00007ffeae1d29e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 1.809775] RAX: ffffffffffffffda RBX: 000055909ee6aee0 RCX: 00007f2312b50469 [ 1.810159] RDX: 0000000000000000 RSI: 00007f2313469265 RDI: 0000000000000010 [ 1.810543] RBP: 00007f2313469265 R08: 0000000000000000 R09: 00007ffeae1d2f60 [ 1.810927] R10: 0000000000000010 R11: 0000000000000246 R12: 0000000000000000 [ 1.811312] R13: 000055909ee6afe0 R14: 0000000000020000 R15: 000055909dd2fefc [ 1.811697] ---[ end trace ff698e93b7952e56 ]--- [ 1.811962] BUG: unable to handle page fault for address: ffffebde000003c8 [ 1.812341] #PF: supervisor read access in kernel mode [ 1.812621] #PF: error_code(0x0000) - not-present page [ 1.812902] PGD 0 P4D 0 [ 1.813044] Oops: 0000 [#1] SMP NOPTI [ 1.813246] CPU: 0 PID: 182 Comm: systemd-udevd Tainted: G W 5.12.0-rc2+ #78 [ 1.813697] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-4 [ 1.814302] RIP: 0010:kfree+0x5d/0x420 [ 1.814508] Code: db 4d 01 fd 0f 82 c8 03 00 00 49 c7 c4 00 00 00 80 4c 2b 25 e5 f1 1c 01 4d 01 ed [ 1.815518] RSP: 0018:ffffc90000ce3970 EFLAGS: 00010286 [ 1.815803] RAX: 1000000000000401 RBX: 0000000000000000 RCX: 0000000000000000 [ 1.816189] RDX: ffff8881047291b0 RSI: ffffffffc01a72cf RDI: 000000000000f000 [ 1.816575] RBP: ffffc90000ce39a8 R08: ffff88817bc18440 R09: 0000000000000246 [ 1.816961] R10: 0000000000000001 R11: 0000000000000001 R12: ffffebde000003c0 [ 1.817348] R13: 000000008000f000 R14: 0000000000000000 R15: 000000000000f000 [ 1.817734] FS: 00007f2313cd38c0(0000) GS:ffff88817bc00000(0000) knlGS:0000000000000000 [ 1.818170] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1.818483] CR2: ffffebde000003c8 CR3: 0000000104a3c000 CR4: 00000000000006f0 [ 1.818871] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1.819259] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 1.819645] Call Trace: [ 1.819781] adf_vf_isr_resource_free+0x9f/0xb0 [intel_qat] [ 1.820088] adf_vf_isr_resource_alloc+0x14d/0x150 [intel_qat] [ 1.820408] adf_dev_init+0xba/0x140 [intel_qat] [ 1.820664] adf_probe+0x32f/0x370 [qat_c3xxxvf] [ 1.820918] local_pci_probe+0x48/0x80 [ 1.821126] pci_device_probe+0x10f/0x1c0 [ 1.821347] really_probe+0xfb/0x420 [ 1.821544] driver_probe_device+0xe9/0x160 [ 1.821774] device_driver_attach+0x5d/0x70 [ 1.822004] __driver_attach+0x8f/0x150 [ 1.822228] ? device_driver_attach+0x70/0x70 [ 1.822471] bus_for_each_dev+0x7e/0xc0 [ 1.822684] driver_attach+0x1e/0x20 [ 1.822883] bus_add_driver+0x152/0x1f0 [ 1.823098] driver_register+0x74/0xd0 [ 1.823305] ? 0xffffffffc009b000 [ 1.823490] __pci_register_driver+0x54/0x60 [ 1.823725] adfdrv_init+0x34/0x1000 [qat_c3xxxvf] [ 1.823988] do_one_initcall+0x48/0x1d0 [ 1.824200] ? __slab_alloc+0x20/0x40 [ 1.824403] ? kmem_cache_alloc_trace+0x390/0x440 [ 1.824662] ? do_init_module+0x28/0x250 [ 1.824879] do_init_module+0x62/0x250 [ 1.825087] load_module+0x23ee/0x26a0 [ 1.825296] __do_sys_finit_module+0xc2/0x120 [ 1.825535] ? __do_sys_finit_module+0xc2/0x120 [ 1.825785] __x64_sys_finit_module+0x1a/0x20 [ 1.826025] do_syscall_64+0x38/0x90 [ 1.826223] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 1.826500] RIP: 0033:0x7f2312b50469 [ 1.826697] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 [ 1.827708] RSP: 002b:00007ffeae1d29e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 1.828118] RAX: ffffffffffffffda RBX: 000055909ee6aee0 RCX: 00007f2312b50469 [ 1.828504] RDX: 0000000000000000 RSI: 00007f2313469265 RDI: 0000000000000010 [ 1.828890] RBP: 00007f2313469265 R08: 0000000000000000 R09: 00007ffeae1d2f60 [ 1.829276] R10: 0000000000000010 R11: 0000000000000246 R12: 0000000000000000 [ 1.829662] R13: 000055909ee6afe0 R14: 0000000000020000 R15: 000055909dd2fefc [ 1.830049] Modules linked in: qat_c3xxxvf(+) drm snd_pcm intel_qat snd_timer snd joydev psmouse 4 [ 1.831540] CR2: ffffebde000003c8 [ 1.831724] ---[ end trace ff698e93b7952e57 ]--- [ 1.831976] RIP: 0010:kfree+0x5d/0x420 [ 1.832184] Code: db 4d 01 fd 0f 82 c8 03 00 00 49 c7 c4 00 00 00 80 4c 2b 25 e5 f1 1c 01 4d 01 ed [ 1.833214] RSP: 0018:ffffc90000ce3970 EFLAGS: 00010286 [ 1.833503] RAX: 1000000000000401 RBX: 0000000000000000 RCX: 0000000000000000 [ 1.833894] RDX: ffff8881047291b0 RSI: ffffffffc01a72cf RDI: 000000000000f000 [ 1.834286] RBP: ffffc90000ce39a8 R08: ffff88817bc18440 R09: 0000000000000246 [ 1.834677] R10: 0000000000000001 R11: 0000000000000001 R12: ffffebde000003c0 [ 1.835068] R13: 000000008000f000 R14: 0000000000000000 R15: 000000000000f000 [ 1.835462] FS: 00007f2313cd38c0(0000) GS:ffff88817bc00000(0000) knlGS:0000000000000000 [ 1.835906] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1.836222] CR2: ffffebde000003c8 CR3: 0000000104a3c000 CR4: 00000000000006f0 [ 1.836614] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1.837003] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Signed-off-by: Tong Zhang <ztong0001@gmail.com> --- drivers/crypto/qat/qat_common/adf_vf_isr.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c index 38d316a42ba6..888388acb6bd 100644 --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c @@ -261,17 +261,26 @@ int adf_vf_isr_resource_alloc(struct adf_accel_dev *accel_dev) goto err_out; if (adf_setup_pf2vf_bh(accel_dev)) - goto err_out; + goto err_disable_msi; if (adf_setup_bh(accel_dev)) - goto err_out; + goto err_cleanup_pf2vf_bh; if (adf_request_msi_irq(accel_dev)) - goto err_out; + goto err_cleanup_bh; return 0; + +err_cleanup_bh: + adf_cleanup_bh(accel_dev); + +err_cleanup_pf2vf_bh: + adf_cleanup_pf2vf_bh(accel_dev); + +err_disable_msi: + adf_disable_msi(accel_dev); + err_out: - adf_vf_isr_resource_free(accel_dev); return -EFAULT; } EXPORT_SYMBOL_GPL(adf_vf_isr_resource_alloc); -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-12 7:43 [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-12 7:43 ` [PATCH 1/2] crypto: qat - dont release uninitialized resources Tong Zhang @ 2021-03-12 7:43 ` Tong Zhang 2021-03-12 8:55 ` Andy Shevchenko 2021-03-12 11:32 ` [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Andy Shevchenko 2 siblings, 1 reply; 6+ messages in thread From: Tong Zhang @ 2021-03-12 7:43 UTC (permalink / raw) To: Giovanni Cabiddu, Herbert Xu, David S. Miller, Wojciech Ziemba, Andy Shevchenko, Fiona Trahe, Tong Zhang, qat-linux, linux-crypto, linux-kernel ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the vf2pf_lock is initialized in adf_dev_init(), which can fail and when it fail, the vf2pf_lock is either not initialized or destroyed, a subsequent use of vf2pf_lock will cause issue. To fix this issue, only set this flag if adf_dev_init() returns 0. [ 7.178008] ================================================================== [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 [ 7.178800] Read of size 4 at addr 000000000000f434 by task modprobe/96 [ 7.179169] [ 7.179257] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-00338-gf78d76e72a46-dirty #86 [ 7.179730] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-4 [ 7.180345] Call Trace: [ 7.180485] dump_stack+0x8a/0xb5 [ 7.180676] kasan_report.cold+0x10f/0x111 [ 7.180907] ? __mutex_lock.isra.0+0x1ac/0x7c0 [ 7.181156] __mutex_lock.isra.0+0x1ac/0x7c0 [ 7.181394] ? finish_task_switch+0x12f/0x360 [ 7.181640] ? __switch_to+0x339/0x6b0 [ 7.181852] ? ww_mutex_lock_interruptible+0x150/0x150 [ 7.182138] ? __sched_text_start+0x8/0x8 [ 7.182363] ? vprintk_emit+0x91/0x170 [ 7.182576] mutex_lock+0xc9/0xd0 [ 7.182765] ? __mutex_lock_slowpath+0x10/0x10 [ 7.183016] ? swsusp_write.cold+0x208/0x208 [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] [ 7.184403] ? adf_remove+0x70/0x70 [qat_dh895xccvf] [ 7.184681] ? _raw_spin_lock_irqsave+0x7b/0xd0 [ 7.184936] ? _raw_spin_unlock_irqrestore+0xd/0x20 [ 7.185209] ? adf_remove+0x70/0x70 [qat_dh895xccvf] [ 7.185489] local_pci_probe+0x6f/0xb0 [ 7.185702] pci_device_probe+0x1e9/0x2f0 [ 7.185928] ? pci_device_remove+0xf0/0xf0 [ 7.186159] ? sysfs_do_create_link_sd.isra.0+0x76/0xe0 [ 7.186458] really_probe+0x161/0x420 [ 7.186665] driver_probe_device+0x6d/0xd0 [ 7.186894] device_driver_attach+0x82/0x90 [ 7.187131] ? device_driver_attach+0x90/0x90 [ 7.187375] __driver_attach+0x60/0x100 [ 7.187591] ? device_driver_attach+0x90/0x90 [ 7.187835] bus_for_each_dev+0xe1/0x140 [ 7.188057] ? subsys_dev_iter_exit+0x10/0x10 [ 7.188302] ? klist_node_init+0x61/0x80 [ 7.188524] bus_add_driver+0x254/0x2a0 [ 7.188740] driver_register+0xd3/0x150 [ 7.188956] ? 0xffffffffc0050000 [ 7.189143] adfdrv_init+0x2b/0x1000 [qat_dh895xccvf] [ 7.189427] do_one_initcall+0x84/0x250 [ 7.189644] ? trace_event_raw_event_initcall_finish+0x150/0x150 [ 7.189977] ? _raw_spin_unlock_irqrestore+0xd/0x20 [ 7.190250] ? create_object+0x395/0x510 [ 7.190472] ? kasan_unpoison+0x21/0x50 [ 7.190689] do_init_module+0xf8/0x350 [ 7.190901] load_module+0x40c5/0x4410 [ 7.191121] ? module_frob_arch_sections+0x20/0x20 [ 7.191390] ? kernel_read_file+0x1cd/0x3e0 [ 7.191626] ? __do_sys_finit_module+0x108/0x170 [ 7.191885] __do_sys_finit_module+0x108/0x170 [ 7.192134] ? __ia32_sys_init_module+0x40/0x40 [ 7.192389] ? file_open_root+0x200/0x200 [ 7.192615] ? do_sys_open+0x85/0xe0 [ 7.192817] ? filp_open+0x50/0x50 [ 7.193010] ? exit_to_user_mode_prepare+0xfc/0x130 [ 7.193283] do_syscall_64+0x33/0x40 [ 7.193486] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 7.193770] RIP: 0033:0x7f6ad21b4cf7 [ 7.193971] Code: 48 89 57 30 48 8b 04 24 48 89 47 38 e9 1d a0 02 00 48 89 f8 48 89 f7 48 89 d6 41 [ 7.194991] RSP: 002b:00007ffe2a5d9028 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 7.195408] RAX: ffffffffffffffda RBX: 0000000000882a70 RCX: 00007f6ad21b4cf7 [ 7.195801] RDX: 0000000000000000 RSI: 00000000008819e0 RDI: 0000000000000003 [ 7.196193] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000001 [ 7.196588] R10: 00007f6ad2218300 R11: 0000000000000246 R12: 00000000008819e0 [ 7.196979] R13: 0000000000000000 R14: 0000000000881dd0 R15: 0000000000000001 [ 7.197372] ================================================================== Signed-off-by: Tong Zhang <ztong0001@gmail.com> --- drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c index 1d1532e8fb6d..067ca5e17d38 100644 --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c index 04742a6d91ca..51ea88c0b17d 100644 --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c index c972554a755e..29999da716cc 100644 --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_err_free_reg; - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); - ret = adf_dev_init(accel_dev); if (ret) goto out_err_dev_shutdown; + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); + ret = adf_dev_start(accel_dev); if (ret) goto out_err_dev_stop; -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-12 7:43 ` [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang @ 2021-03-12 8:55 ` Andy Shevchenko 2021-03-12 14:20 ` Tong Zhang 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2021-03-12 8:55 UTC (permalink / raw) To: Tong Zhang Cc: Giovanni Cabiddu, Herbert Xu, David S. Miller, Wojciech Ziemba, Andy Shevchenko, Fiona Trahe, qat-linux, linux-crypto, Linux Kernel Mailing List On Fri, Mar 12, 2021 at 9:50 AM Tong Zhang <ztong0001@gmail.com> wrote: > > ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() > before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the > vf2pf_lock is initialized in adf_dev_init(), which can fail and when it > fail, the vf2pf_lock is either not initialized or destroyed, a subsequent > use of vf2pf_lock will cause issue. > To fix this issue, only set this flag if adf_dev_init() returns 0. Makes sense, but please leave only ~2-3 (significant) lines from the below noisy dump Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> (after making commit message neat) > [ 7.178008] ================================================================== > [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 > [ 7.178800] Read of size 4 at addr 000000000000f434 by task modprobe/96 > [ 7.179169] > [ 7.179257] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-00338-gf78d76e72a46-dirty #86 > [ 7.179730] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-4 > [ 7.180345] Call Trace: > [ 7.180485] dump_stack+0x8a/0xb5 > [ 7.180676] kasan_report.cold+0x10f/0x111 > [ 7.180907] ? __mutex_lock.isra.0+0x1ac/0x7c0 > [ 7.181156] __mutex_lock.isra.0+0x1ac/0x7c0 > [ 7.181394] ? finish_task_switch+0x12f/0x360 > [ 7.181640] ? __switch_to+0x339/0x6b0 > [ 7.181852] ? ww_mutex_lock_interruptible+0x150/0x150 > [ 7.182138] ? __sched_text_start+0x8/0x8 > [ 7.182363] ? vprintk_emit+0x91/0x170 > [ 7.182576] mutex_lock+0xc9/0xd0 > [ 7.182765] ? __mutex_lock_slowpath+0x10/0x10 > [ 7.183016] ? swsusp_write.cold+0x208/0x208 > [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] > [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] > [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] > [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] > [ 7.184403] ? adf_remove+0x70/0x70 [qat_dh895xccvf] > [ 7.184681] ? _raw_spin_lock_irqsave+0x7b/0xd0 > [ 7.184936] ? _raw_spin_unlock_irqrestore+0xd/0x20 > [ 7.185209] ? adf_remove+0x70/0x70 [qat_dh895xccvf] > [ 7.185489] local_pci_probe+0x6f/0xb0 > [ 7.185702] pci_device_probe+0x1e9/0x2f0 > [ 7.185928] ? pci_device_remove+0xf0/0xf0 > [ 7.186159] ? sysfs_do_create_link_sd.isra.0+0x76/0xe0 > [ 7.186458] really_probe+0x161/0x420 > [ 7.186665] driver_probe_device+0x6d/0xd0 > [ 7.186894] device_driver_attach+0x82/0x90 > [ 7.187131] ? device_driver_attach+0x90/0x90 > [ 7.187375] __driver_attach+0x60/0x100 > [ 7.187591] ? device_driver_attach+0x90/0x90 > [ 7.187835] bus_for_each_dev+0xe1/0x140 > [ 7.188057] ? subsys_dev_iter_exit+0x10/0x10 > [ 7.188302] ? klist_node_init+0x61/0x80 > [ 7.188524] bus_add_driver+0x254/0x2a0 > [ 7.188740] driver_register+0xd3/0x150 > [ 7.188956] ? 0xffffffffc0050000 > [ 7.189143] adfdrv_init+0x2b/0x1000 [qat_dh895xccvf] > [ 7.189427] do_one_initcall+0x84/0x250 > [ 7.189644] ? trace_event_raw_event_initcall_finish+0x150/0x150 > [ 7.189977] ? _raw_spin_unlock_irqrestore+0xd/0x20 > [ 7.190250] ? create_object+0x395/0x510 > [ 7.190472] ? kasan_unpoison+0x21/0x50 > [ 7.190689] do_init_module+0xf8/0x350 > [ 7.190901] load_module+0x40c5/0x4410 > [ 7.191121] ? module_frob_arch_sections+0x20/0x20 > [ 7.191390] ? kernel_read_file+0x1cd/0x3e0 > [ 7.191626] ? __do_sys_finit_module+0x108/0x170 > [ 7.191885] __do_sys_finit_module+0x108/0x170 > [ 7.192134] ? __ia32_sys_init_module+0x40/0x40 > [ 7.192389] ? file_open_root+0x200/0x200 > [ 7.192615] ? do_sys_open+0x85/0xe0 > [ 7.192817] ? filp_open+0x50/0x50 > [ 7.193010] ? exit_to_user_mode_prepare+0xfc/0x130 > [ 7.193283] do_syscall_64+0x33/0x40 > [ 7.193486] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 7.193770] RIP: 0033:0x7f6ad21b4cf7 > [ 7.193971] Code: 48 89 57 30 48 8b 04 24 48 89 47 38 e9 1d a0 02 00 48 89 f8 48 89 f7 48 89 d6 41 > [ 7.194991] RSP: 002b:00007ffe2a5d9028 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > [ 7.195408] RAX: ffffffffffffffda RBX: 0000000000882a70 RCX: 00007f6ad21b4cf7 > [ 7.195801] RDX: 0000000000000000 RSI: 00000000008819e0 RDI: 0000000000000003 > [ 7.196193] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000001 > [ 7.196588] R10: 00007f6ad2218300 R11: 0000000000000246 R12: 00000000008819e0 > [ 7.196979] R13: 0000000000000000 R14: 0000000000881dd0 R15: 0000000000000001 > [ 7.197372] ================================================================== > > Signed-off-by: Tong Zhang <ztong0001@gmail.com> > --- > drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- > drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > index 1d1532e8fb6d..067ca5e17d38 100644 > --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > index 04742a6d91ca..51ea88c0b17d 100644 > --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > index c972554a755e..29999da716cc 100644 > --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (ret) > goto out_err_free_reg; > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > - > ret = adf_dev_init(accel_dev); > if (ret) > goto out_err_dev_shutdown; > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > + > ret = adf_dev_start(accel_dev); > if (ret) > goto out_err_dev_stop; > -- > 2.25.1 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init 2021-03-12 8:55 ` Andy Shevchenko @ 2021-03-12 14:20 ` Tong Zhang 0 siblings, 0 replies; 6+ messages in thread From: Tong Zhang @ 2021-03-12 14:20 UTC (permalink / raw) To: Andy Shevchenko Cc: Giovanni Cabiddu, Herbert Xu, David S. Miller, Wojciech Ziemba, Andy Shevchenko, Fiona Trahe, qat-linux, linux-crypto, Linux Kernel Mailing List Hi Andy, Thanks for the suggestion. I have removed excessive lines in the commit log and sent out a v2 patch. Thanks again. - Tong On Fri, Mar 12, 2021 at 3:56 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Fri, Mar 12, 2021 at 9:50 AM Tong Zhang <ztong0001@gmail.com> wrote: > > > > ADF_STATUS_PF_RUNNING is (only) used and checked by adf_vf2pf_shutdown() > > before calling adf_iov_putmsg()->mutex_lock(vf2pf_lock), however the > > vf2pf_lock is initialized in adf_dev_init(), which can fail and when it > > fail, the vf2pf_lock is either not initialized or destroyed, a subsequent > > use of vf2pf_lock will cause issue. > > To fix this issue, only set this flag if adf_dev_init() returns 0. > > Makes sense, but please leave only ~2-3 (significant) lines from the > below noisy dump > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > (after making commit message neat) > > > [ 7.178008] ================================================================== > > [ 7.178404] BUG: KASAN: user-memory-access in __mutex_lock.isra.0+0x1ac/0x7c0 > > [ 7.178800] Read of size 4 at addr 000000000000f434 by task modprobe/96 > > [ 7.179169] > > [ 7.179257] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-00338-gf78d76e72a46-dirty #86 > > [ 7.179730] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-4 > > [ 7.180345] Call Trace: > > [ 7.180485] dump_stack+0x8a/0xb5 > > [ 7.180676] kasan_report.cold+0x10f/0x111 > > [ 7.180907] ? __mutex_lock.isra.0+0x1ac/0x7c0 > > [ 7.181156] __mutex_lock.isra.0+0x1ac/0x7c0 > > [ 7.181394] ? finish_task_switch+0x12f/0x360 > > [ 7.181640] ? __switch_to+0x339/0x6b0 > > [ 7.181852] ? ww_mutex_lock_interruptible+0x150/0x150 > > [ 7.182138] ? __sched_text_start+0x8/0x8 > > [ 7.182363] ? vprintk_emit+0x91/0x170 > > [ 7.182576] mutex_lock+0xc9/0xd0 > > [ 7.182765] ? __mutex_lock_slowpath+0x10/0x10 > > [ 7.183016] ? swsusp_write.cold+0x208/0x208 > > [ 7.183257] adf_iov_putmsg+0x118/0x1a0 [intel_qat] > > [ 7.183541] adf_vf2pf_shutdown+0x4d/0x7b [intel_qat] > > [ 7.183834] adf_dev_shutdown+0x172/0x2b0 [intel_qat] > > [ 7.184127] adf_probe+0x5e9/0x600 [qat_dh895xccvf] > > [ 7.184403] ? adf_remove+0x70/0x70 [qat_dh895xccvf] > > [ 7.184681] ? _raw_spin_lock_irqsave+0x7b/0xd0 > > [ 7.184936] ? _raw_spin_unlock_irqrestore+0xd/0x20 > > [ 7.185209] ? adf_remove+0x70/0x70 [qat_dh895xccvf] > > [ 7.185489] local_pci_probe+0x6f/0xb0 > > [ 7.185702] pci_device_probe+0x1e9/0x2f0 > > [ 7.185928] ? pci_device_remove+0xf0/0xf0 > > [ 7.186159] ? sysfs_do_create_link_sd.isra.0+0x76/0xe0 > > [ 7.186458] really_probe+0x161/0x420 > > [ 7.186665] driver_probe_device+0x6d/0xd0 > > [ 7.186894] device_driver_attach+0x82/0x90 > > [ 7.187131] ? device_driver_attach+0x90/0x90 > > [ 7.187375] __driver_attach+0x60/0x100 > > [ 7.187591] ? device_driver_attach+0x90/0x90 > > [ 7.187835] bus_for_each_dev+0xe1/0x140 > > [ 7.188057] ? subsys_dev_iter_exit+0x10/0x10 > > [ 7.188302] ? klist_node_init+0x61/0x80 > > [ 7.188524] bus_add_driver+0x254/0x2a0 > > [ 7.188740] driver_register+0xd3/0x150 > > [ 7.188956] ? 0xffffffffc0050000 > > [ 7.189143] adfdrv_init+0x2b/0x1000 [qat_dh895xccvf] > > [ 7.189427] do_one_initcall+0x84/0x250 > > [ 7.189644] ? trace_event_raw_event_initcall_finish+0x150/0x150 > > [ 7.189977] ? _raw_spin_unlock_irqrestore+0xd/0x20 > > [ 7.190250] ? create_object+0x395/0x510 > > [ 7.190472] ? kasan_unpoison+0x21/0x50 > > [ 7.190689] do_init_module+0xf8/0x350 > > [ 7.190901] load_module+0x40c5/0x4410 > > [ 7.191121] ? module_frob_arch_sections+0x20/0x20 > > [ 7.191390] ? kernel_read_file+0x1cd/0x3e0 > > [ 7.191626] ? __do_sys_finit_module+0x108/0x170 > > [ 7.191885] __do_sys_finit_module+0x108/0x170 > > [ 7.192134] ? __ia32_sys_init_module+0x40/0x40 > > [ 7.192389] ? file_open_root+0x200/0x200 > > [ 7.192615] ? do_sys_open+0x85/0xe0 > > [ 7.192817] ? filp_open+0x50/0x50 > > [ 7.193010] ? exit_to_user_mode_prepare+0xfc/0x130 > > [ 7.193283] do_syscall_64+0x33/0x40 > > [ 7.193486] entry_SYSCALL_64_after_hwframe+0x44/0xae > > [ 7.193770] RIP: 0033:0x7f6ad21b4cf7 > > [ 7.193971] Code: 48 89 57 30 48 8b 04 24 48 89 47 38 e9 1d a0 02 00 48 89 f8 48 89 f7 48 89 d6 41 > > [ 7.194991] RSP: 002b:00007ffe2a5d9028 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > > [ 7.195408] RAX: ffffffffffffffda RBX: 0000000000882a70 RCX: 00007f6ad21b4cf7 > > [ 7.195801] RDX: 0000000000000000 RSI: 00000000008819e0 RDI: 0000000000000003 > > [ 7.196193] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000001 > > [ 7.196588] R10: 00007f6ad2218300 R11: 0000000000000246 R12: 00000000008819e0 > > [ 7.196979] R13: 0000000000000000 R14: 0000000000881dd0 R15: 0000000000000001 > > [ 7.197372] ================================================================== > > > > Signed-off-by: Tong Zhang <ztong0001@gmail.com> > > --- > > drivers/crypto/qat/qat_c3xxxvf/adf_drv.c | 4 ++-- > > drivers/crypto/qat/qat_c62xvf/adf_drv.c | 4 ++-- > > drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 4 ++-- > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > > index 1d1532e8fb6d..067ca5e17d38 100644 > > --- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > > +++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c > > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (ret) > > goto out_err_free_reg; > > > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > - > > ret = adf_dev_init(accel_dev); > > if (ret) > > goto out_err_dev_shutdown; > > > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > + > > ret = adf_dev_start(accel_dev); > > if (ret) > > goto out_err_dev_stop; > > diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > > index 04742a6d91ca..51ea88c0b17d 100644 > > --- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c > > +++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c > > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (ret) > > goto out_err_free_reg; > > > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > - > > ret = adf_dev_init(accel_dev); > > if (ret) > > goto out_err_dev_shutdown; > > > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > + > > ret = adf_dev_start(accel_dev); > > if (ret) > > goto out_err_dev_stop; > > diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > > index c972554a755e..29999da716cc 100644 > > --- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > > +++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c > > @@ -184,12 +184,12 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (ret) > > goto out_err_free_reg; > > > > - set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > - > > ret = adf_dev_init(accel_dev); > > if (ret) > > goto out_err_dev_shutdown; > > > > + set_bit(ADF_STATUS_PF_RUNNING, &accel_dev->status); > > + > > ret = adf_dev_start(accel_dev); > > if (ret) > > goto out_err_dev_stop; > > -- > > 2.25.1 > > > > > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] crypto: qat: fix couple crashes duing error handling 2021-03-12 7:43 [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-12 7:43 ` [PATCH 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 2021-03-12 7:43 ` [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang @ 2021-03-12 11:32 ` Andy Shevchenko 2 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2021-03-12 11:32 UTC (permalink / raw) To: Tong Zhang Cc: Giovanni Cabiddu, Herbert Xu, David S. Miller, Wojciech Ziemba, Fiona Trahe, qat-linux, linux-crypto, linux-kernel On Fri, Mar 12, 2021 at 02:43:56AM -0500, Tong Zhang wrote: > There are a couple of issues in qat error handling. Those drivers tries to > release resources that is not initialized. This patch series tries to fix > crashes caused by incorrect error handling. Please, in this series and everywhere else use a common sense, when put backtraces, oopses, etc in the commit message, i.e. leave only ~2-3 (significant) lines instead of 20-30. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-12 14:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-12 7:43 [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Tong Zhang 2021-03-12 7:43 ` [PATCH 1/2] crypto: qat - dont release uninitialized resources Tong Zhang 2021-03-12 7:43 ` [PATCH 2/2] crypto: qat: ADF_STATUS_PF_RUNNING should be set after adf_dev_init Tong Zhang 2021-03-12 8:55 ` Andy Shevchenko 2021-03-12 14:20 ` Tong Zhang 2021-03-12 11:32 ` [PATCH 0/2] crypto: qat: fix couple crashes duing error handling Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome