Re: [Linux bug] WARNING in quota_release_workfn

[Jan Kara]

 On Fri 20-02-26 23:26:55, Sam Sun wrote:
> Dear developers and maintainers,
> 
> We hit the following WARNING while running a modified syzkaller on
> v6.19 (commit 2961f841b025). We use the kernel config on syzbot to
> compile the kernel
> (https://syzkaller.appspot.com/text?tag=KernelConfig&x=e2f061f80b102378),
> unfortunately no reproducer is available now. The bug was previously
> reported by syzbot and marked as invalid due to no more occurrence
> (https://syzkaller.appspot.com/bug?extid=0b3a51c4b82c0d16d60d):
> 
> ------------[ cut here ]------------
> atomic_read(&dquot->dq_count)
> WARNING: fs/quota/dquot.c:829 at quota_release_workfn+0x6cf/0x980
> fs/quota/dquot.c:829, CPU#1: kworker/u10:7/11898
> Modules linked in:
> CPU: 1 UID: 0 PID: 11898 Comm: kworker/u10:7 Tainted: G             L
>     6.19.0-11564-g2961f841b025-dirty #18 PREEMPT(full)
> Tainted: [L]=SOFTLOCKUP
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> Workqueue: quota_events_unbound quota_release_workfn
> RIP: 0010:quota_release_workfn+0x6cf/0x980 fs/quota/dquot.c:829
> Code: c3 cc cc cc cc e8 21 63 5b ff be 08 00 00 00 4c 89 e7 e8 84 cf
> c5 ff f0 80 a3 10 01 00 00 bf e9 c2 fe ff ff e8 02 63 5b ff 90 <0f> 0b
> 90 e9 ca fa ff ff e8 f4 62 5b ff 48 c7 c7 40 6a 21 8e e8 08
> RSP: 0018:ffa00000099e7b98 EFLAGS: 00010293
> RAX: 0000000000000000 RBX: ff110000418f9620 RCX: ffffffff82637d58
> RDX: ff1100011034a4c0 RSI: ffffffff8263828e RDI: 0000000000000005
> RBP: dffffc0000000000 R08: 0000000000000000 R09: ffe21c000831f2e2
> R10: 0000000000000002 R11: 0000000000000086 R12: 0000000000000002
> R13: ffffffff90b7aa54 R14: 0000000000000001 R15: ff110000418f9600
> FS:  0000000000000000(0000) GS:ff110001a1195000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007ff39e0723f0 CR3: 0000000027620000 CR4: 0000000000753ef0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  process_one_work+0x9fb/0x1d00 kernel/workqueue.c:3349
>  process_scheduled_works kernel/workqueue.c:3448 [inline]
>  worker_thread+0x67e/0xe90 kernel/workqueue.c:3529
>  kthread+0x38d/0x4a0 kernel/kthread.c:467
>  ret_from_fork+0xb32/0xde0 arch/x86/kernel/process.c:158
>  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
>  </TASK>
> 
> <<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>
> 
> 
> The WARN_ON_ONCE at dquot.c:829 fires inside quota_release_workfn() when
> a dquot on the releasing list has dq_count != 0:
> 
>     WARN_ON_ONCE(atomic_read(&dquot->dq_count));
> 
> We analyzed the root cause of this warning, and doubt this is a race
> between dquot_scan_active() and quota_release_workfn(). dqput() drops
> dq_count to 0 immediately (while DQ_ACTIVE_B is still set) and marks
> the dquot with DQ_RELEASING_B. Several places were updated to check
> DQ_RELEASING_B accordingly (invalidate_dquots,
> dquot_writeback_dquots), but dquot_scan_active() was not.
> 
> The race window is as follows:
> 
>   CPU0 (quota_release_workfn)         CPU1 (dquot_scan_active)
>   ==============================      ==============================
>   spin_lock(&dq_list_lock);
>   list_replace_init(
>     &releasing_dquots, &rls_head);
>     /* dquot X on rls_head,
>        dq_count == 0,
>        DQ_ACTIVE_B still set */
>   spin_unlock(&dq_list_lock);
>   synchronize_srcu(&dquot_srcu);
>     /* takes a while... */
>                                       spin_lock(&dq_list_lock);
>                                       list_for_each_entry(dquot,
>                                           &inuse_list, dq_inuse) {
>                                         /* finds dquot X */
>                                         dquot_active(X) -> true
>                                         /* DQ_RELEASING_B not checked! */
>                                         atomic_inc(&X->dq_count);
>                                         /* X still on rls_head,
>                                            dq_count is now 1 */
>                                       }
>                                       spin_unlock(&dq_list_lock);
>   /* srcu done */
>   spin_lock(&dq_list_lock);
>   dquot = list_first_entry(&rls_head);
>   WARN_ON_ONCE(atomic_read(
>     &dquot->dq_count));
>     /* dq_count == 1 -> WARN! */
> 
> dquot_scan_active() increments dq_count on a dquot it finds via
> inuse_list without checking DQ_RELEASING_B and without calling
> remove_free_dquot(). The dquot thus remains on the worker's rls_head
> list with a non-zero reference count.
> 
> A possible fix could add a DQ_RELEASING_B check to dquot_scan_active(), similar
> to what was done for invalidate_dquots() and dquot_writeback_dquots() in
> commit 869b6ea1609f. Something like:

Thanks for report! Yes, your analysis looks correct. After some thought
I've ended up with attached fix.

								Honza

> 
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -639,6 +639,14 @@ int dquot_scan_active(struct super_block *sb,
>          if (dquot->dq_sb != sb)
>              continue;
> - /* Now we have active dquot so we can just increase use count */
> + /*
> + * dquot is being released via quota_release_workfn().
> + * Skip it - it will be cleaned up by the worker.
> + */
> + if (test_bit(DQ_RELEASING_B, &dquot->dq_flags))
> + continue;
> + if (!atomic_read(&dquot->dq_count))
> + remove_free_dquot(dquot);
> + /* Now we have active dquot, increase use count */
>          atomic_inc(&dquot->dq_count);
> 
> 
> If you have any questions, please let me know.
> 
> Best Regards,
> Yue
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR