btrfs: fix potential deadlock in the search ioctl
authorJosef Bacik <josef@toxicpanda.com>
Mon, 10 Aug 2020 15:42:27 +0000 (11:42 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 12 Sep 2020 09:47:35 +0000 (11:47 +0200)
commit52245f066ca7027acc270050e6b3916059ee21c7
tree5cf617cb26a294b8d0dfd2dd2633c286382649e9
parent6faf75bacc39732aac1785340f9bf959a2425699
btrfs: fix potential deadlock in the search ioctl

[ Upstream commit a48b73eca4ceb9b8a4b97f290a065335dbcd8a04 ]

With the conversion of the tree locks to rwsem I got the following
lockdep splat:

  ======================================================
  WARNING: possible circular locking dependency detected
  5.8.0-rc7-00165-g04ec4da5f45f-dirty #922 Not tainted
  ------------------------------------------------------
  compsize/11122 is trying to acquire lock:
  ffff889fabca8768 (&mm->mmap_lock#2){++++}-{3:3}, at: __might_fault+0x3e/0x90

  but task is already holding lock:
  ffff889fe720fe40 (btrfs-fs-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x39/0x180

  which lock already depends on the new lock.

  the existing dependency chain (in reverse order) is:

  -> #2 (btrfs-fs-00){++++}-{3:3}:
 down_write_nested+0x3b/0x70
 __btrfs_tree_lock+0x24/0x120
 btrfs_search_slot+0x756/0x990
 btrfs_lookup_inode+0x3a/0xb4
 __btrfs_update_delayed_inode+0x93/0x270
 btrfs_async_run_delayed_root+0x168/0x230
 btrfs_work_helper+0xd4/0x570
 process_one_work+0x2ad/0x5f0
 worker_thread+0x3a/0x3d0
 kthread+0x133/0x150
 ret_from_fork+0x1f/0x30

  -> #1 (&delayed_node->mutex){+.+.}-{3:3}:
 __mutex_lock+0x9f/0x930
 btrfs_delayed_update_inode+0x50/0x440
 btrfs_update_inode+0x8a/0xf0
 btrfs_dirty_inode+0x5b/0xd0
 touch_atime+0xa1/0xd0
 btrfs_file_mmap+0x3f/0x60
 mmap_region+0x3a4/0x640
 do_mmap+0x376/0x580
 vm_mmap_pgoff+0xd5/0x120
 ksys_mmap_pgoff+0x193/0x230
 do_syscall_64+0x50/0x90
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

  -> #0 (&mm->mmap_lock#2){++++}-{3:3}:
 __lock_acquire+0x1272/0x2310
 lock_acquire+0x9e/0x360
 __might_fault+0x68/0x90
 _copy_to_user+0x1e/0x80
 copy_to_sk.isra.32+0x121/0x300
 search_ioctl+0x106/0x200
 btrfs_ioctl_tree_search_v2+0x7b/0xf0
 btrfs_ioctl+0x106f/0x30a0
 ksys_ioctl+0x83/0xc0
 __x64_sys_ioctl+0x16/0x20
 do_syscall_64+0x50/0x90
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

  other info that might help us debug this:

  Chain exists of:
    &mm->mmap_lock#2 --> &delayed_node->mutex --> btrfs-fs-00

   Possible unsafe locking scenario:

 CPU0                    CPU1
 ----                    ----
    lock(btrfs-fs-00);
 lock(&delayed_node->mutex);
 lock(btrfs-fs-00);
    lock(&mm->mmap_lock#2);

   *** DEADLOCK ***

  1 lock held by compsize/11122:
   #0: ffff889fe720fe40 (btrfs-fs-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x39/0x180

  stack backtrace:
  CPU: 17 PID: 11122 Comm: compsize Kdump: loaded Not tainted 5.8.0-rc7-00165-g04ec4da5f45f-dirty #922
  Hardware name: Quanta Tioga Pass Single Side 01-0030993006/Tioga Pass Single Side, BIOS F08_3A18 12/20/2018
  Call Trace:
   dump_stack+0x78/0xa0
   check_noncircular+0x165/0x180
   __lock_acquire+0x1272/0x2310
   lock_acquire+0x9e/0x360
   ? __might_fault+0x3e/0x90
   ? find_held_lock+0x72/0x90
   __might_fault+0x68/0x90
   ? __might_fault+0x3e/0x90
   _copy_to_user+0x1e/0x80
   copy_to_sk.isra.32+0x121/0x300
   ? btrfs_search_forward+0x2a6/0x360
   search_ioctl+0x106/0x200
   btrfs_ioctl_tree_search_v2+0x7b/0xf0
   btrfs_ioctl+0x106f/0x30a0
   ? __do_sys_newfstat+0x5a/0x70
   ? ksys_ioctl+0x83/0xc0
   ksys_ioctl+0x83/0xc0
   __x64_sys_ioctl+0x16/0x20
   do_syscall_64+0x50/0x90
   entry_SYSCALL_64_after_hwframe+0x44/0xa9

The problem is we're doing a copy_to_user() while holding tree locks,
which can deadlock if we have to do a page fault for the copy_to_user().
This exists even without my locking changes, so it needs to be fixed.
Rework the search ioctl to do the pre-fault and then
copy_to_user_nofault for the copying.

CC: stable@vger.kernel.org # 4.4+
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
fs/btrfs/extent_io.c
fs/btrfs/extent_io.h
fs/btrfs/ioctl.c