Commit 09c81391 authored by Baokun Li's avatar Baokun Li
Browse files

fs/dcache: fix bad unlock balance in shrink_dentry_list()

hulk inclusion
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/IBW902



--------------------------------

Inside shrink_lock_dentry(), if we can't lock the parent, we unlock the
dentry and wait for the parent lock. Once we get the parent lock, we lock
the dentry again. After locking the dentry a second time, we only check
d_lockref.count and don't see if d_inode changed.

But during that lock gap, d_inode may be modified. If the inode changes,
the inode in the dentry isn't locked. However, in dentry_unlink_inode(),
we unlock the inode no matter what. Then, we will get the following Oops:

------------[ cut here ]------------
pvqspinlock: lock 0xff438c8e84836be0 has corrupted value 0x0!
WARNING: CPU: 1 PID: 370 at kernel/locking/qspinlock_paravirt.h:498
CPU: 1 PID: 370 Comm: bash Not tainted 5.10.0 #2008
RIP: 0010:__pv_queued_spin_unlock_slowpath+0xbf/0xd0
Call Trace:
 __raw_callee_save___pv_queued_spin_unlock_slowpath+0x11/0x24
 .slowpath+0x9/0x16
 dentry_unlink_inode+0x91/0x120
 __dentry_kill+0xf3/0x1c0
 shrink_dentry_list+0x4b/0xe0
 prune_dcache_sb+0x52/0x80
 super_cache_scan+0x133/0x1f0
 do_shrink_slab+0x14a/0x2f0

Here's how concurrency might trigger this issue:

        P1           |          P2
---------------------------------------------
 prune_dcache_sb
  shrink_dentry_list
   spin_lock(&dentry->d_lock)
   shrink_lock_dentry
    /* Negative dentry, therefore no inode to lock. ref 0 */
    spin_trylock(&parent->d_lock) / try lock failed /
    spin_unlock(&dentry->d_lock)
                      lookup_open
                        __d_lookup
                         spin_lock(&dentry->d_lock)
                         dentry->d_lockref.count++ /* ref 1 */
                         spin_unlock(&dentry->d_lock)
                       /* Negative dentry, just create the file */
                       xfs_generic_create
                        d_instantiate
                         spin_lock(&inode->i_lock)
                          __d_instantiate
                           spin_lock(&dentry->d_lock)
                           __d_set_inode_and_type
                            dentry->d_inode = inode
                           spin_unlock(&dentry->d_lock)
                         spin_unlock(&inode->i_lock)
                      syscall_exit_to_user_mode /* Task exit. */
                         __fput
                          dput
                           fast_dput
                            spin_lock(&dentry->d_lock)
                           retain_dentry
                            dentry->d_lockref.count-- /* ref 0 */
                           spin_unlock(&dentry->d_lock)
    spin_lock(&parent->d_lock)
    spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED)
    /* d_inode has been changed but is not locked. */
   __dentry_kill
    dentry_unlink_inode
     spin_unlock(&dentry->d_lock)
     spin_unlock(&inode->i_lock)
     /* Attempt to unlock an unlocked inode. */
     /* WARNING: bad unlock balance in dentry_unlink_inode */

Therefore, add the missing d_inode check to avoid this issue.

Fixes: 3b3f09f4 ("get rid of trylock loop in locking dentries on shrink list")
Signed-off-by: default avatarBaokun Li <libaokun1@huawei.com>
parent 1d4ddaa5
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -1174,7 +1174,7 @@ static bool shrink_lock_dentry(struct dentry *dentry)
		goto out;
	}
	spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
	if (likely(!dentry->d_lockref.count))
	if (likely(!dentry->d_lockref.count && inode == dentry->d_inode))
		return true;
	spin_unlock(&parent->d_lock);
out: