[PATCH] ext4: fix deadlock in ext4_evict_ea_inode() vs cache_find()

[Yun Zhou]

ext4_evict_ea_inode() calls mb_cache_entry_wait_unused() to wait for
the EA inode's mb_cache entry refcount to drop before deleting it.
This can deadlock against ext4_xattr_inode_cache_find() which holds
an mb_cache entry reference while calling ext4_iget():

  Thread A (evicting EA inode X):
    ext4_evict_ea_inode -> mb_cache_entry_wait_unused  [waits for refcnt]

  Thread B (setxattr looking for dedup):
    ext4_xattr_inode_cache_find -> mb_cache_entry_find  [holds refcnt]
      -> ext4_iget(X) -> __wait_on_freeing_inode       [waits on I_FREEING]

Thread A's eviction sets I_FREEING and then waits for the mb_cache
refcount to drop.  Thread B holds that refcount and waits for
I_FREEING to clear -- circular dependency.

Fix by making ext4_evict_ea_inode() non-blocking: try to delete the
entry once, and if it's currently in use, just leave it.  A stale
entry is harmless -- subsequent cache_find() calls will iget the
evicted inode, fail validation or data comparison, and skip it.

Fixes: 458aee4a6e5b ("ext4: remove EA inode entry from mbcache on inode eviction")
Reported-by: syzbot+fd5533bcd0f7343bb8ca@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=fd5533bcd0f7343bb8ca
Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>
---
 fs/ext4/xattr.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 85ad2561474b..8ca2c87b1db3 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -471,12 +471,16 @@ void ext4_evict_ea_inode(struct inode *inode)
 
 	if (!EA_INODE_CACHE(inode))
 		return;
-	/* Wait for entry to get unused so that we can remove it */
-	while ((oe = mb_cache_entry_delete_or_get(EA_INODE_CACHE(inode),
-			ext4_xattr_inode_get_hash(inode), inode->i_ino))) {
-		mb_cache_entry_wait_unused(oe);
+	/*
+	 * Try to delete the cache entry.  If it's currently in use by
+	 * another thread (e.g. ext4_xattr_inode_cache_find), just leave
+	 * it -- the stale entry is harmless.  Waiting here would deadlock
+	 * if the other thread's iget is blocked on this inode's I_FREEING.
+	 */
+	oe = mb_cache_entry_delete_or_get(EA_INODE_CACHE(inode),
+			ext4_xattr_inode_get_hash(inode), inode->i_ino);
+	if (oe)
 		mb_cache_entry_put(EA_INODE_CACHE(inode), oe);
-	}
 }
 
 static int
-- 
2.43.0