It can reproduced by the following commands:
# modprobe pcrypt # echo 2 > /sys/kernel/pcrypt/pencrypt/parallel_cpumask # echo 0 > /sys/devices/system/cpu/cpu1/online # modprobe tcrypt alg="pcrypt(rfc4106(gcm(aes)))" type=3
[ 229.549005] divide error: 0000 [#1] SMP PTI [ 229.549130] CPU: 32 PID: 9565 Comm: cryptomgr_test Kdump: loaded Not tainted 4.19.200 #3 [ 229.549381] Hardware name: Huawei 2288H V5/BC11SPSCB0, BIOS 0.68 05/03/2018 [ 229.549607] RIP: 0010:padata_do_parallel+0x96/0x150 [ 229.549750] Code: 5e 10 89 56 18 f0 0f c1 6b 20 8b 35 78 b1 24 01 48 8b 7b 28 e8 eb d6 20 00 89 c1 8d 45 01 31 d2 8b 35 62 b1 24 01 48 8b 7b 28 <f7> f1 41 89 d7 e8 d0 45 21 00 45 85 ff 41 89 c4 7e 19 31 ed 48 8b [ 229.550335] RSP: 0018:ffffa48b8e1cbbc8 EFLAGS: 00010246 [ 229.550498] RAX: 0000000000000000 RBX: ffff964940883bc0 RCX: 0000000000000000 [ 229.550720] RDX: 0000000000000000 RSI: 0000000000000038 RDI: ffff9687b6e45650 [ 229.550943] RBP: 00000000ffffffff R08: 0000000000000000 R09: ffff96c7af6f6200 [ 229.551165] R10: ffff9687b6e45650 R11: ffff968839629000 R12: 0000000000000010 [ 229.551388] R13: ffff9687b6997f50 R14: ffff96c7ad84d700 R15: ffffffff9287a220 [ 229.551610] FS: 0000000000000000(0000) GS:ffff9687bfc80000(0000) knlGS:0000000000000000 [ 229.551863] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 229.552044] CR2: 00007fad13d47564 CR3: 000000759480a004 CR4: 00000000007606e0 [ 229.552265] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 229.552488] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 229.552710] PKRU: 55555554 [ 229.552796] Call Trace: [ 229.552878] pcrypt_aead_encrypt+0xbb/0xc7 [pcrypt] [ 229.553028] __test_aead+0x654/0x15d0 [ 229.553139] ? _cond_resched+0x15/0x40 [ 229.553257] ? crypto_create_tfm+0x4e/0xe0 [ 229.553385] ? crypto_spawn_tfm2+0x2e/0x50 [ 229.553513] ? _cond_resched+0x15/0x40 [ 229.553632] ? crypto_acomp_scomp_free_ctx+0x30/0x30 [ 229.553786] test_aead+0x21/0xa0 [ 229.553889] alg_test_aead+0x3f/0xa0 [ 229.554001] alg_test.part.15+0x178/0x380 [ 229.554127] ? __switch_to+0x8c/0x400 [ 229.554239] ? __switch_to_asm+0x41/0x70 [ 229.554362] ? __switch_to_asm+0x35/0x70 [ 229.554486] ? __schedule+0x25d/0x850 [ 229.554602] ? __wake_up_common+0x76/0x170 [ 229.554727] ? crypto_acomp_scomp_free_ctx+0x30/0x30 [ 229.554884] cryptomgr_test+0x40/0x50 [ 229.554999] kthread+0x113/0x130 [ 229.555099] ? kthread_create_worker_on_cpu+0x70/0x70 [ 229.555255] ret_from_fork+0x35/0x40
Daniel Jordan (2): padata: validate cpumask without removed CPU during offline padata: add separate cpuhp node for CPUHP_PADATA_DEAD
include/linux/cpuhotplug.h | 1 + include/linux/padata.h | 6 ++++-- kernel/padata.c | 28 ++++++++++++++++++++-------- 3 files changed, 25 insertions(+), 10 deletions(-)
From: Daniel Jordan daniel.m.jordan@oracle.com
[ Upstream commit 894c9ef9780c5cf2f143415e867ee39a33ecb75d ]
Configuring an instance's parallel mask without any online CPUs...
echo 2 > /sys/kernel/pcrypt/pencrypt/parallel_cpumask echo 0 > /sys/devices/system/cpu/cpu1/online
...makes tcrypt mode=215 crash like this:
divide error: 0000 [#1] SMP PTI CPU: 4 PID: 283 Comm: modprobe Not tainted 5.4.0-rc8-padata-doc-v2+ #2 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20191013_105130-anatol 04/01/2014 RIP: 0010:padata_do_parallel+0x114/0x300 Call Trace: pcrypt_aead_encrypt+0xc0/0xd0 [pcrypt] crypto_aead_encrypt+0x1f/0x30 do_mult_aead_op+0x4e/0xdf [tcrypt] test_mb_aead_speed.constprop.0.cold+0x226/0x564 [tcrypt] do_test+0x28c2/0x4d49 [tcrypt] tcrypt_mod_init+0x55/0x1000 [tcrypt] ...
cpumask_weight() in padata_cpu_hash() returns 0 because the mask has no CPUs. The problem is __padata_remove_cpu() checks for valid masks too early and so doesn't mark the instance PADATA_INVALID as expected, which would have made padata_do_parallel() return error before doing the division.
Fix by introducing a second padata CPU hotplug state before CPUHP_BRINGUP_CPU so that __padata_remove_cpu() sees the online mask without @cpu. No need for the second argument to padata_replace() since @cpu is now already missing from the online mask.
Fixes: 33e54450683c ("padata: Handle empty padata cpumasks") Signed-off-by: Daniel Jordan daniel.m.jordan@oracle.com Cc: Eric Biggers ebiggers@kernel.org Cc: Herbert Xu herbert@gondor.apana.org.au Cc: Sebastian Andrzej Siewior bigeasy@linutronix.de Cc: Steffen Klassert steffen.klassert@secunet.com Cc: Thomas Gleixner tglx@linutronix.de Cc: linux-crypto@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Herbert Xu herbert@gondor.apana.org.au Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- include/linux/cpuhotplug.h | 1 + kernel/padata.c | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h index 3d323c6c85260..b51da879d7be0 100644 --- a/include/linux/cpuhotplug.h +++ b/include/linux/cpuhotplug.h @@ -59,6 +59,7 @@ enum cpuhp_state { CPUHP_IOMMU_INTEL_DEAD, CPUHP_LUSTRE_CFS_DEAD, CPUHP_AP_ARM_CACHE_B15_RAC_DEAD, + CPUHP_PADATA_DEAD, CPUHP_WORKQUEUE_PREP, CPUHP_POWER_NUMA_PREPARE, CPUHP_HRTIMERS_PREPARE, diff --git a/kernel/padata.c b/kernel/padata.c index 93e4fb2d9f2ee..4401b4f13d0be 100644 --- a/kernel/padata.c +++ b/kernel/padata.c @@ -682,7 +682,7 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu) { struct parallel_data *pd = NULL;
- if (cpumask_test_cpu(cpu, cpu_online_mask)) { + if (!cpumask_test_cpu(cpu, cpu_online_mask)) {
if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) || !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu)) @@ -758,7 +758,7 @@ static int padata_cpu_online(unsigned int cpu, struct hlist_node *node) return ret; }
-static int padata_cpu_prep_down(unsigned int cpu, struct hlist_node *node) +static int padata_cpu_dead(unsigned int cpu, struct hlist_node *node) { struct padata_instance *pinst; int ret; @@ -779,6 +779,7 @@ static enum cpuhp_state hp_online; static void __padata_free(struct padata_instance *pinst) { #ifdef CONFIG_HOTPLUG_CPU + cpuhp_state_remove_instance_nocalls(CPUHP_PADATA_DEAD, &pinst->node); cpuhp_state_remove_instance_nocalls(hp_online, &pinst->node); #endif
@@ -964,6 +965,8 @@ static struct padata_instance *padata_alloc(struct workqueue_struct *wq,
#ifdef CONFIG_HOTPLUG_CPU cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, &pinst->node); + cpuhp_state_add_instance_nocalls_cpuslocked(CPUHP_PADATA_DEAD, + &pinst->node); #endif return pinst;
@@ -1010,17 +1013,24 @@ static __init int padata_driver_init(void) int ret;
ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "padata:online", - padata_cpu_online, - padata_cpu_prep_down); + padata_cpu_online, NULL); if (ret < 0) return ret; hp_online = ret; + + ret = cpuhp_setup_state_multi(CPUHP_PADATA_DEAD, "padata:dead", + NULL, padata_cpu_dead); + if (ret < 0) { + cpuhp_remove_multi_state(hp_online); + return ret; + } return 0; } module_init(padata_driver_init);
static __exit void padata_driver_exit(void) { + cpuhp_remove_multi_state(CPUHP_PADATA_DEAD); cpuhp_remove_multi_state(hp_online); } module_exit(padata_driver_exit);
From: Daniel Jordan daniel.m.jordan@oracle.com
[ Upstream commit 3c2214b6027ff37945799de717c417212e1a8c54 ]
Removing the pcrypt module triggers this:
general protection fault, probably for non-canonical address 0xdead000000000122 CPU: 5 PID: 264 Comm: modprobe Not tainted 5.6.0+ #2 Hardware name: QEMU Standard PC RIP: 0010:__cpuhp_state_remove_instance+0xcc/0x120 Call Trace: padata_sysfs_release+0x74/0xce kobject_put+0x81/0xd0 padata_free+0x12/0x20 pcrypt_exit+0x43/0x8ee [pcrypt]
padata instances wrongly use the same hlist node for the online and dead states, so __padata_free()'s second cpuhp remove call chokes on the node that the first poisoned.
cpuhp multi-instance callbacks only walk forward in cpuhp_step->list and the same node is linked in both the online and dead lists, so the list corruption that results from padata_alloc() adding the node to a second list without removing it from the first doesn't cause problems as long as no instances are freed.
Avoid the issue by giving each state its own node.
Fixes: 894c9ef9780c ("padata: validate cpumask without removed CPU during offline") Signed-off-by: Daniel Jordan daniel.m.jordan@oracle.com Cc: Herbert Xu herbert@gondor.apana.org.au Cc: Steffen Klassert steffen.klassert@secunet.com Cc: linux-crypto@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org # v5.4+ Signed-off-by: Herbert Xu herbert@gondor.apana.org.au Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- include/linux/padata.h | 6 ++++-- kernel/padata.c | 14 ++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/include/linux/padata.h b/include/linux/padata.h index d803397a28f70..8c9827cc63747 100644 --- a/include/linux/padata.h +++ b/include/linux/padata.h @@ -138,7 +138,8 @@ struct parallel_data { /** * struct padata_instance - The overall control structure. * - * @cpu_notifier: cpu hotplug notifier. + * @cpu_online_node: Linkage for CPU online callback. + * @cpu_dead_node: Linkage for CPU offline callback. * @wq: The workqueue in use. * @pd: The internal control structure. * @cpumask: User supplied cpumasks for parallel and serial works. @@ -150,7 +151,8 @@ struct parallel_data { * @flags: padata flags. */ struct padata_instance { - struct hlist_node node; + struct hlist_node cpu_online_node; + struct hlist_node cpu_dead_node; struct workqueue_struct *wq; struct parallel_data *pd; struct padata_cpumask cpumask; diff --git a/kernel/padata.c b/kernel/padata.c index 4401b4f13d0be..7f2b6d369fd47 100644 --- a/kernel/padata.c +++ b/kernel/padata.c @@ -748,7 +748,7 @@ static int padata_cpu_online(unsigned int cpu, struct hlist_node *node) struct padata_instance *pinst; int ret;
- pinst = hlist_entry_safe(node, struct padata_instance, node); + pinst = hlist_entry_safe(node, struct padata_instance, cpu_online_node); if (!pinst_has_cpu(pinst, cpu)) return 0;
@@ -763,7 +763,7 @@ static int padata_cpu_dead(unsigned int cpu, struct hlist_node *node) struct padata_instance *pinst; int ret;
- pinst = hlist_entry_safe(node, struct padata_instance, node); + pinst = hlist_entry_safe(node, struct padata_instance, cpu_dead_node); if (!pinst_has_cpu(pinst, cpu)) return 0;
@@ -779,8 +779,9 @@ static enum cpuhp_state hp_online; static void __padata_free(struct padata_instance *pinst) { #ifdef CONFIG_HOTPLUG_CPU - cpuhp_state_remove_instance_nocalls(CPUHP_PADATA_DEAD, &pinst->node); - cpuhp_state_remove_instance_nocalls(hp_online, &pinst->node); + cpuhp_state_remove_instance_nocalls(CPUHP_PADATA_DEAD, + &pinst->cpu_dead_node); + cpuhp_state_remove_instance_nocalls(hp_online, &pinst->cpu_online_node); #endif
padata_stop(pinst); @@ -964,9 +965,10 @@ static struct padata_instance *padata_alloc(struct workqueue_struct *wq, mutex_init(&pinst->lock);
#ifdef CONFIG_HOTPLUG_CPU - cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, &pinst->node); + cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, + &pinst->cpu_online_node); cpuhp_state_add_instance_nocalls_cpuslocked(CPUHP_PADATA_DEAD, - &pinst->node); + &pinst->cpu_dead_node); #endif return pinst;
On Tue, Aug 03, 2021 at 08:52:59PM +0800, Yang Yingliang wrote:
It can reproduced by the following commands:
# modprobe pcrypt # echo 2 > /sys/kernel/pcrypt/pencrypt/parallel_cpumask # echo 0 > /sys/devices/system/cpu/cpu1/online # modprobe tcrypt alg="pcrypt(rfc4106(gcm(aes)))" type=3
[ 229.549005] divide error: 0000 [#1] SMP PTI [ 229.549130] CPU: 32 PID: 9565 Comm: cryptomgr_test Kdump: loaded Not tainted 4.19.200 #3 [ 229.549381] Hardware name: Huawei 2288H V5/BC11SPSCB0, BIOS 0.68 05/03/2018 [ 229.549607] RIP: 0010:padata_do_parallel+0x96/0x150 [ 229.549750] Code: 5e 10 89 56 18 f0 0f c1 6b 20 8b 35 78 b1 24 01 48 8b 7b 28 e8 eb d6 20 00 89 c1 8d 45 01 31 d2 8b 35 62 b1 24 01 48 8b 7b 28 <f7> f1 41 89 d7 e8 d0 45 21 00 45 85 ff 41 89 c4 7e 19 31 ed 48 8b [ 229.550335] RSP: 0018:ffffa48b8e1cbbc8 EFLAGS: 00010246 [ 229.550498] RAX: 0000000000000000 RBX: ffff964940883bc0 RCX: 0000000000000000 [ 229.550720] RDX: 0000000000000000 RSI: 0000000000000038 RDI: ffff9687b6e45650 [ 229.550943] RBP: 00000000ffffffff R08: 0000000000000000 R09: ffff96c7af6f6200 [ 229.551165] R10: ffff9687b6e45650 R11: ffff968839629000 R12: 0000000000000010 [ 229.551388] R13: ffff9687b6997f50 R14: ffff96c7ad84d700 R15: ffffffff9287a220 [ 229.551610] FS: 0000000000000000(0000) GS:ffff9687bfc80000(0000) knlGS:0000000000000000 [ 229.551863] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 229.552044] CR2: 00007fad13d47564 CR3: 000000759480a004 CR4: 00000000007606e0 [ 229.552265] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 229.552488] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 229.552710] PKRU: 55555554 [ 229.552796] Call Trace: [ 229.552878] pcrypt_aead_encrypt+0xbb/0xc7 [pcrypt] [ 229.553028] __test_aead+0x654/0x15d0 [ 229.553139] ? _cond_resched+0x15/0x40 [ 229.553257] ? crypto_create_tfm+0x4e/0xe0 [ 229.553385] ? crypto_spawn_tfm2+0x2e/0x50 [ 229.553513] ? _cond_resched+0x15/0x40 [ 229.553632] ? crypto_acomp_scomp_free_ctx+0x30/0x30 [ 229.553786] test_aead+0x21/0xa0 [ 229.553889] alg_test_aead+0x3f/0xa0 [ 229.554001] alg_test.part.15+0x178/0x380 [ 229.554127] ? __switch_to+0x8c/0x400 [ 229.554239] ? __switch_to_asm+0x41/0x70 [ 229.554362] ? __switch_to_asm+0x35/0x70 [ 229.554486] ? __schedule+0x25d/0x850 [ 229.554602] ? __wake_up_common+0x76/0x170 [ 229.554727] ? crypto_acomp_scomp_free_ctx+0x30/0x30 [ 229.554884] cryptomgr_test+0x40/0x50 [ 229.554999] kthread+0x113/0x130 [ 229.555099] ? kthread_create_worker_on_cpu+0x70/0x70 [ 229.555255] ret_from_fork+0x35/0x40
Daniel Jordan (2): padata: validate cpumask without removed CPU during offline padata: add separate cpuhp node for CPUHP_PADATA_DEAD
include/linux/cpuhotplug.h | 1 + include/linux/padata.h | 6 ++++-- kernel/padata.c | 28 ++++++++++++++++++++-------- 3 files changed, 25 insertions(+), 10 deletions(-)
All now queued up, thanks!
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Yang Yingliang