[PATCH v2] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()

[Aditya Srivastava]

From: Aditya Prakash Srivastava <aditya.ansh182@xxxxxxxxx>

Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
concurrent xattr workloads (using the ea_inode mount/format option).

The deadlock occurs between the running transaction and the eviction
thread:
- Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
  and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
  the corresponding EA inode. Since the EA inode is currently being
  evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
  eviction to complete.
- Task 2 (eviction thread): Currently evicting the same EA inode in
  ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
  blocks waiting for Task 1 to release the reference to the mbcache_entry.

To break this deadlock, perform a non-blocking lookup of the EA inode
using VFS's find_inode_nowait() API. If the EA inode is currently being
evicted (marked with I_FREEING or I_WILL_FREE), simply skip it (treat
as a cache miss) rather than waiting for eviction to complete. If the
returned inode is found to be I_NEW, wait for its initialization to
clear using wait_on_new_inode().

This deadlock was made much easier to hit after commit 0a46ef234756
("ext4: do not create EA inode under buffer lock") which removed
synchronization on the buffer lock.

Reported-by: Colin Ian King <colin.i.king@xxxxxxxxx>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@xxxxxxxxx>
---
Changes in v2:
  - Read inode state locklessly using inode_state_read_once() to resolve
    a lockdep assertion on cache hit.
  - Manually restore essential inode/ea_inode validations on the retrieved
    inode (is_bad_inode, EXT4_EA_INODE_FL, file_acl, and xattr checks) to
    match VFS safety guarantees and prevent using corrupted/failed inodes.

 fs/ext4/xattr.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e22..ef13e7a76153 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1523,6 +1523,20 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
 	return ea_inode;
 }
 
+static int ext4_xattr_inode_match(struct inode *inode, u64 ino, void *data)
+{
+	if (inode->i_ino != ino)
+		return 0;
+	spin_lock(&inode->i_lock);
+	if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
+		spin_unlock(&inode->i_lock);
+		return 0;
+	}
+	__iget(inode);
+	spin_unlock(&inode->i_lock);
+	return 1;
+}
+
 static struct inode *
 ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
 			    size_t value_len, u32 hash)
@@ -1549,10 +1563,19 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
 	}
 
 	while (ce) {
-		ea_inode = ext4_iget(inode->i_sb, ce->e_value,
-				     EXT4_IGET_EA_INODE);
-		if (IS_ERR(ea_inode))
+		ea_inode = find_inode_nowait(inode->i_sb, ce->e_value,
+					     ext4_xattr_inode_match, NULL);
+		if (!ea_inode)
 			goto next_entry;
+		if (inode_state_read_once(ea_inode) & I_NEW)
+			wait_on_new_inode(ea_inode);
+		if (is_bad_inode(ea_inode) ||
+		    !(EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL) ||
+		    ext4_test_inode_state(ea_inode, EXT4_STATE_XATTR) ||
+		    EXT4_I(ea_inode)->i_file_acl) {
+			iput(ea_inode);
+			goto next_entry;
+		}
 		ext4_xattr_inode_set_class(ea_inode);
 		if (i_size_read(ea_inode) == value_len &&
 		    !ext4_xattr_inode_read(ea_inode, ea_data, value_len) &&
-- 
2.47.3