• Josef Bacik's avatar
    btrfs: fix potential deadlock in the search ioctl · a48b73ec
    Josef Bacik authored
    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: default avatarFilipe Manana <fdmanana@suse.com>
    Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
    Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
    Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
    a48b73ec
extent_io.h 10.7 KB