From: Josef Bacik <josef@toxicpanda.com>
To: fdmanana@kernel.org
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: release path before inode lookup during the ino lookup ioctl
Date: Mon, 28 Aug 2023 09:12:37 -0400 [thread overview]
Message-ID: <20230828131237.GA875235@perftesting> (raw)
In-Reply-To: <3433395cd3c3c880bf01392d07813d3677fd010e.1693045620.git.fdmanana@suse.com>
On Sat, Aug 26, 2023 at 11:28:20AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> During the ino lookup ioctl we can end up calling btrfs_iget() to get an
> inode reference while we are holding on a root's btree. If btrfs_iget()
> needs to lookup the inode from the root's btree, because it's not
> currently loaded in memory, then it will need to lock another or the
> same path in the same root btree. This may result in a deadlock and
> trigger the following lockdep splat:
>
> WARNING: possible circular locking dependency detected
> 6.5.0-rc7-syzkaller-00004-gf7757129e3de #0 Not tainted
> ------------------------------------------------------
> syz-executor277/5012 is trying to acquire lock:
> ffff88802df41710 (btrfs-tree-01){++++}-{3:3}, at: __btrfs_tree_read_lock+0x2f/0x220 fs/btrfs/locking.c:136
>
> but task is already holding lock:
> ffff88802df418e8 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x2f/0x220 fs/btrfs/locking.c:136
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (btrfs-tree-00){++++}-{3:3}:
> down_read_nested+0x49/0x2f0 kernel/locking/rwsem.c:1645
> __btrfs_tree_read_lock+0x2f/0x220 fs/btrfs/locking.c:136
> btrfs_search_slot+0x13a4/0x2f80 fs/btrfs/ctree.c:2302
> btrfs_init_root_free_objectid+0x148/0x320 fs/btrfs/disk-io.c:4955
> btrfs_init_fs_root fs/btrfs/disk-io.c:1128 [inline]
> btrfs_get_root_ref+0x5ae/0xae0 fs/btrfs/disk-io.c:1338
> btrfs_get_fs_root fs/btrfs/disk-io.c:1390 [inline]
> open_ctree+0x29c8/0x3030 fs/btrfs/disk-io.c:3494
> btrfs_fill_super+0x1c7/0x2f0 fs/btrfs/super.c:1154
> btrfs_mount_root+0x7e0/0x910 fs/btrfs/super.c:1519
> legacy_get_tree+0xef/0x190 fs/fs_context.c:611
> vfs_get_tree+0x8c/0x270 fs/super.c:1519
> fc_mount fs/namespace.c:1112 [inline]
> vfs_kern_mount+0xbc/0x150 fs/namespace.c:1142
> btrfs_mount+0x39f/0xb50 fs/btrfs/super.c:1579
> legacy_get_tree+0xef/0x190 fs/fs_context.c:611
> vfs_get_tree+0x8c/0x270 fs/super.c:1519
> do_new_mount+0x28f/0xae0 fs/namespace.c:3335
> do_mount fs/namespace.c:3675 [inline]
> __do_sys_mount fs/namespace.c:3884 [inline]
> __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3861
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> -> #0 (btrfs-tree-01){++++}-{3:3}:
> check_prev_add kernel/locking/lockdep.c:3142 [inline]
> check_prevs_add kernel/locking/lockdep.c:3261 [inline]
> validate_chain kernel/locking/lockdep.c:3876 [inline]
> __lock_acquire+0x39ff/0x7f70 kernel/locking/lockdep.c:5144
> lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5761
> down_read_nested+0x49/0x2f0 kernel/locking/rwsem.c:1645
> __btrfs_tree_read_lock+0x2f/0x220 fs/btrfs/locking.c:136
> btrfs_tree_read_lock fs/btrfs/locking.c:142 [inline]
> btrfs_read_lock_root_node+0x292/0x3c0 fs/btrfs/locking.c:281
> btrfs_search_slot_get_root fs/btrfs/ctree.c:1832 [inline]
> btrfs_search_slot+0x4ff/0x2f80 fs/btrfs/ctree.c:2154
> btrfs_lookup_inode+0xdc/0x480 fs/btrfs/inode-item.c:412
> btrfs_read_locked_inode fs/btrfs/inode.c:3892 [inline]
> btrfs_iget_path+0x2d9/0x1520 fs/btrfs/inode.c:5716
> btrfs_search_path_in_tree_user fs/btrfs/ioctl.c:1961 [inline]
> btrfs_ioctl_ino_lookup_user+0x77a/0xf50 fs/btrfs/ioctl.c:2105
> btrfs_ioctl+0xb0b/0xd40 fs/btrfs/ioctl.c:4683
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:870 [inline]
> __se_sys_ioctl+0xf8/0x170 fs/ioctl.c:856
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> rlock(btrfs-tree-00);
> lock(btrfs-tree-01);
> lock(btrfs-tree-00);
> rlock(btrfs-tree-01);
>
> *** DEADLOCK ***
>
> 1 lock held by syz-executor277/5012:
> #0: ffff88802df418e8 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x2f/0x220 fs/btrfs/locking.c:136
>
> stack backtrace:
> CPU: 1 PID: 5012 Comm: syz-executor277 Not tainted 6.5.0-rc7-syzkaller-00004-gf7757129e3de #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/26/2023
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
> check_noncircular+0x375/0x4a0 kernel/locking/lockdep.c:2195
> check_prev_add kernel/locking/lockdep.c:3142 [inline]
> check_prevs_add kernel/locking/lockdep.c:3261 [inline]
> validate_chain kernel/locking/lockdep.c:3876 [inline]
> __lock_acquire+0x39ff/0x7f70 kernel/locking/lockdep.c:5144
> lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5761
> down_read_nested+0x49/0x2f0 kernel/locking/rwsem.c:1645
> __btrfs_tree_read_lock+0x2f/0x220 fs/btrfs/locking.c:136
> btrfs_tree_read_lock fs/btrfs/locking.c:142 [inline]
> btrfs_read_lock_root_node+0x292/0x3c0 fs/btrfs/locking.c:281
> btrfs_search_slot_get_root fs/btrfs/ctree.c:1832 [inline]
> btrfs_search_slot+0x4ff/0x2f80 fs/btrfs/ctree.c:2154
> btrfs_lookup_inode+0xdc/0x480 fs/btrfs/inode-item.c:412
> btrfs_read_locked_inode fs/btrfs/inode.c:3892 [inline]
> btrfs_iget_path+0x2d9/0x1520 fs/btrfs/inode.c:5716
> btrfs_search_path_in_tree_user fs/btrfs/ioctl.c:1961 [inline]
> btrfs_ioctl_ino_lookup_user+0x77a/0xf50 fs/btrfs/ioctl.c:2105
> btrfs_ioctl+0xb0b/0xd40 fs/btrfs/ioctl.c:4683
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:870 [inline]
> __se_sys_ioctl+0xf8/0x170 fs/ioctl.c:856
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7f0bec94ea39
>
> Fix this simply by releasing the path before calling btrfs_iget() as at
> point we don't need the path anymore.
>
> Reported-by: syzbot+bf66ad948981797d2f1d@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/linux-btrfs/00000000000045fa140603c4a969@google.com/
> Fixes: 23d0b79dfaed ("btrfs: Add unprivileged version of ino_lookup ioctl")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Thanks,
Josef
next prev parent reply other threads:[~2023-08-28 13:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-26 10:28 [PATCH] btrfs: release path before inode lookup during the ino lookup ioctl fdmanana
2023-08-28 13:12 ` Josef Bacik [this message]
2023-09-04 21:41 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230828131237.GA875235@perftesting \
--to=josef@toxicpanda.com \
--cc=fdmanana@kernel.org \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox