2020-09-02 17:10:21

by Daniel Jordan

Subject: [PATCH] padata: fix possible padata_works_lock deadlock

syzbot reports,

WARNING: inconsistent lock state
5.9.0-rc2-syzkaller #0 Not tainted
--------------------------------
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
syz-executor.0/26715 takes:
(padata_works_lock){+.?.}-{2:2}, at: padata_do_parallel kernel/padata.c:220
{IN-SOFTIRQ-W} state was registered at:
spin_lock include/linux/spinlock.h:354 [inline]
padata_do_parallel kernel/padata.c:220
...
__do_softirq kernel/softirq.c:298
...
sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1091
asm_sysvec_apic_timer_interrupt arch/x86/include/asm/idtentry.h:581

Possible unsafe locking scenario:

CPU0
----
lock(padata_works_lock);
<Interrupt>
lock(padata_works_lock);

padata_do_parallel() takes padata_works_lock with softirqs enabled, so a
deadlock is possible if, on the same CPU, the lock is acquired in
process context and then softirq handling done in an interrupt leads to
the same path.

Fix by leaving softirqs disabled while do_parallel holds
padata_works_lock.

Reported-by: [email protected]
Fixes: 4611ce2246889 ("padata: allocate work structures for parallel jobs from a pool")
Signed-off-by: Daniel Jordan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Steffen Klassert <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
kernel/padata.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/padata.c b/kernel/padata.c
index 16cb894dc272..d4d3ba6e1728 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -215,12 +215,13 @@ int padata_do_parallel(struct padata_shell *ps,
padata->pd = pd;
padata->cb_cpu = *cb_cpu;

- rcu_read_unlock_bh();
-
spin_lock(&padata_works_lock);
padata->seq_nr = ++pd->seq_nr;
pw = padata_work_alloc();
spin_unlock(&padata_works_lock);
+
+ rcu_read_unlock_bh();
+
if (pw) {
padata_work_init(pw, padata_parallel_worker, padata, 0);
queue_work(pinst->parallel_wq, &pw->pw_work);

base-commit: 9c7d619be5a002ea29c172df5e3c1227c22cbb41
--
2.28.0

2020-09-04 08:31:06

by Herbert Xu

Subject: Re: [PATCH] padata: fix possible padata_works_lock deadlock

On Wed, Sep 02, 2020 at 01:07:56PM -0400, Daniel Jordan wrote:
> syzbot reports,
>
> WARNING: inconsistent lock state
> 5.9.0-rc2-syzkaller #0 Not tainted
> --------------------------------
> inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> syz-executor.0/26715 takes:
> (padata_works_lock){+.?.}-{2:2}, at: padata_do_parallel kernel/padata.c:220
> {IN-SOFTIRQ-W} state was registered at:
> spin_lock include/linux/spinlock.h:354 [inline]
> padata_do_parallel kernel/padata.c:220
> ...
> __do_softirq kernel/softirq.c:298
> ...
> sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1091
> asm_sysvec_apic_timer_interrupt arch/x86/include/asm/idtentry.h:581
>
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(padata_works_lock);
> <Interrupt>
> lock(padata_works_lock);
>
> padata_do_parallel() takes padata_works_lock with softirqs enabled, so a
> deadlock is possible if, on the same CPU, the lock is acquired in
> process context and then softirq handling done in an interrupt leads to
> the same path.
>
> Fix by leaving softirqs disabled while do_parallel holds
> padata_works_lock.
>
> Reported-by: [email protected]
> Fixes: 4611ce2246889 ("padata: allocate work structures for parallel jobs from a pool")
> Signed-off-by: Daniel Jordan <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: Steffen Klassert <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> kernel/padata.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt