MIME-Version: 1.0
References: <000000000000f6540d05f30bb23f@google.com> <20230126121905.toze65yum336s42p@quack3>
In-Reply-To: <20230126121905.toze65yum336s42p@quack3>
From:   Dmitry Vyukov <dvyukov@google.com>
Date:   Fri, 27 Jan 2023 08:13:32 +0100
Message-ID: <CACT4Y+ZV0t9vSgwurRGDsOTdBQJO1ZyOA2s0JkgL1oKXchQjSA@mail.gmail.com>
Subject: Re: [syzbot] [ext4?] possible deadlock in ext4_xattr_set_handle (3)
To:     Jan Kara <jack@suse.cz>
Cc:     syzbot <syzbot+edce54daffee36421b4c@syzkaller.appspotmail.com>,
        adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
        linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
        tytso@mit.edu
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 26 Jan 2023 at 13:19, Jan Kara <jack@suse.cz> wrote:
>
> Hi!
>
> On Tue 24-01-23 16:27:36, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit:    edc00350d205 Merge tag 'block-6.2-2023-01-20' of git://git..
> > git tree:       upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=134b1441480000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=899d86a7610a0ea0
> > dashboard link: https://syzkaller.appspot.com/bug?extid=edce54daffee36421b4c
> > compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > userspace arch: i386
> >
> > Unfortunately, I don't have any reproducer for this issue yet.
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+edce54daffee36421b4c@syzkaller.appspotmail.com
> >
> > ext4 filesystem being mounted at /syzkaller-testdir3627507797/syzkaller.9jT2hR/316/file0 supports timestamps until 2038 (0x7fffffff)
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 6.2.0-rc4-syzkaller-00350-gedc00350d205 #0 Not tainted
> > ------------------------------------------------------
> > syz-executor.2/573 is trying to acquire lock:
> > ffffffff8c8d4f60 (fs_reclaim){+.+.}-{0:0}, at: might_alloc include/linux/sched/mm.h:271 [inline]
> > ffffffff8c8d4f60 (fs_reclaim){+.+.}-{0:0}, at: slab_pre_alloc_hook mm/slab.h:720 [inline]
> > ffffffff8c8d4f60 (fs_reclaim){+.+.}-{0:0}, at: slab_alloc_node mm/slub.c:3434 [inline]
> > ffffffff8c8d4f60 (fs_reclaim){+.+.}-{0:0}, at: __kmem_cache_alloc_node+0x41/0x430 mm/slub.c:3491
> >
> > but task is already holding lock:
> > ffff8880277eb2f0 (&ei->xattr_sem){++++}-{3:3}, at: ext4_write_lock_xattr fs/ext4/xattr.h:155 [inline]
> > ffff8880277eb2f0 (&ei->xattr_sem){++++}-{3:3}, at: ext4_xattr_set_handle+0x160/0x1510 fs/ext4/xattr.c:2305
> >
> > which lock already depends on the new lock.
>
> So I don't see how the below is ever possible:
>
> > -> #0 (fs_reclaim){+.+.}-{0:0}:
> >        check_prev_add kernel/locking/lockdep.c:3097 [inline]
> >        check_prevs_add kernel/locking/lockdep.c:3216 [inline]
> >        validate_chain kernel/locking/lockdep.c:3831 [inline]
> >        __lock_acquire+0x2a43/0x56d0 kernel/locking/lockdep.c:5055
> >        lock_acquire kernel/locking/lockdep.c:5668 [inline]
> >        lock_acquire+0x1e3/0x630 kernel/locking/lockdep.c:5633
> >        __fs_reclaim_acquire mm/page_alloc.c:4674 [inline]
>
> So we are acquiring fs_reclaim here which means that
> current_gfp_context(gfp_mask) contained __GFP_FS...
>
> >        fs_reclaim_acquire+0x11d/0x160 mm/page_alloc.c:4688
> >        might_alloc include/linux/sched/mm.h:271 [inline]
> >        slab_pre_alloc_hook mm/slab.h:720 [inline]
> >        slab_alloc_node mm/slub.c:3434 [inline]
> >        __kmem_cache_alloc_node+0x41/0x430 mm/slub.c:3491
> >        __do_kmalloc_node mm/slab_common.c:967 [inline]
> >        __kmalloc_node+0x4d/0xd0 mm/slab_common.c:975
> >        kmalloc_node include/linux/slab.h:610 [inline]
> >        kvmalloc_node+0x76/0x1a0 mm/util.c:581
> >        kvmalloc include/linux/slab.h:737 [inline]
> >        ext4_xattr_inode_cache_find fs/ext4/xattr.c:1484 [inline]
> >        ext4_xattr_inode_lookup_create fs/ext4/xattr.c:1527 [inline]
> >        ext4_xattr_set_entry+0x1d92/0x3a00 fs/ext4/xattr.c:1669
> >        ext4_xattr_block_set+0x61b/0x3000 fs/ext4/xattr.c:1906
> >        ext4_xattr_set_handle+0xd8a/0x1510 fs/ext4/xattr.c:2390
> >        ext4_xattr_set+0x144/0x360 fs/ext4/xattr.c:2492
>
> ... however here we've started a transaction so we should have
> PF_MEMALLOC_NOFS set?
>
> The only good explanation I have is that lockdep is mixing dependencies
> from an ext4 filesystem without a journal with dependencies created by
> filesystem with a journal. Since we have no reproducer, it's hard to
> tell.

Should the xattr_sem class be differentiated based on use/no-use of the journal?

> >        __vfs_setxattr+0x173/0x1e0 fs/xattr.c:202
> >        __vfs_setxattr_noperm+0x129/0x5f0 fs/xattr.c:236
> >        __vfs_setxattr_locked+0x1d3/0x260 fs/xattr.c:297
> >        vfs_setxattr+0x143/0x340 fs/xattr.c:323
> >        do_setxattr+0x151/0x190 fs/xattr.c:608
> >        setxattr+0x146/0x160 fs/xattr.c:631
> >        path_setxattr+0x197/0x1c0 fs/xattr.c:650
> >        __do_sys_setxattr fs/xattr.c:666 [inline]
> >        __se_sys_setxattr fs/xattr.c:662 [inline]
> >        __ia32_sys_setxattr+0xc0/0x160 fs/xattr.c:662
> >        do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
> >        __do_fast_syscall_32+0x65/0xf0 arch/x86/entry/common.c:178
> >        do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
> >        entry_SYSENTER_compat_after_hwframe+0x70/0x82
>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR