[PATCH] ext4: compensate ea_inode refs and free block on new xattr block write error

Matthias Goergens <[email protected]>
Newsgroups org.kernel.vger.linux-ext4
Message-ID <[email protected]>
ext4_xattr_block_set() increments the on-disk reference counts of the
ea_inodes a new xattr block refers to before dirtying the block.  If
ext4_handle_dirty_metadata() then fails, the error path neither undoes
the increments nor frees the block.

With a journal the failure aborts the journal and the transaction is
discarded, so nothing leaks.  Without one, ext4_handle_dirty_metadata()
can return -EIO when sync_dirty_buffer() fails for an inode that needs
sync, the increments are already in the dirty ea_inode inode blocks and
reach disk: the reference counts stay one too high forever, the inodes
are never reclaimed, the new block stays allocated but unreferenced,
and the quota charged for the new value is never released.

Compensate on the failure: remove the block from the mbcache, drop the
references, free the block, and release the quota this operation
charged.  A concurrent setxattr may have taken the cached block
meanwhile, so re-check its reference count under the buffer lock and
skip the free if an owner appeared — degrading to the pre-existing
leak rather than corrupting a live owner.

Verified on a nojournal filesystem with ext4_handle_dirty_metadata()
instrumented to return -EIO for the new xattr block (the sync_dirty_
buffer() failure case): unpatched, e2fsck -fn reports the stale
ea_inode reference count, the lost block and the parent's i_blocks
residue; patched, the same injection leaves a clean filesystem and the
no-injection control behaves as before.

Reported-by: Sashiko AI review bot <[email protected]>
Link: https://lore.kernel.org/linux-ext4/[email protected]/
Signed-off-by: Matthias Goergens <[email protected]>
---
 fs/ext4/xattr.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e228..e086c8395042a 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1362,6 +1362,63 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
 	return;
 }
 
+/*
+ * Undo the setup of a new xattr block that failed to be written out: drop
+ * the ext4_xattr_inode_inc_ref_all() references, free the block, and
+ * release the quota this operation charged for the new value (quota_len,
+ * zero when the value is not in an EA inode).
+ */
+static void ext4_xattr_new_block_fail(handle_t *handle, struct inode *inode,
+				      struct buffer_head *new_bh,
+				      size_t quota_len, int error)
+{
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
+	struct mb_cache_entry *oe;
+	struct ext4_xattr_inode_array *ea_inode_array = NULL;
+
+	ext4_error_inode(inode, __func__, __LINE__, 0,
+			 "xattr block dirty failed: %d", error);
+	lock_buffer(new_bh);
+retry_owner:
+	if (le32_to_cpu(BHDR(new_bh)->h_refcount) != 1) {
+		unlock_buffer(new_bh);
+		return;
+	}
+	if (ea_block_cache) {
+		oe = mb_cache_entry_delete_or_get(ea_block_cache,
+						  le32_to_cpu(BHDR(new_bh)->h_hash),
+						  new_bh->b_blocknr);
+		if (oe) {
+			unlock_buffer(new_bh);
+			mb_cache_entry_wait_unused(oe);
+			mb_cache_entry_put(ea_block_cache, oe);
+			lock_buffer(new_bh);
+			goto retry_owner;
+		}
+	}
+	get_bh(new_bh);
+	unlock_buffer(new_bh);
+
+	ext4_xattr_inode_dec_ref_all(handle, inode, new_bh,
+				     ENTRY(BHDR(new_bh) + 1),
+				     true /* block_csum */,
+				     &ea_inode_array,
+				     0 /* extra_credits */,
+				     true /* skip_quota */);
+	ext4_xattr_inode_array_free(ea_inode_array);
+	if (quota_len) {
+		/*
+		 * Reverses this operation's own ext4_xattr_inode_alloc_quota()
+		 * charge, so no EA inode pointer is needed here.
+		 */
+		ext4_xattr_inode_free_quota(inode, NULL, quota_len);
+		ext4_mark_inode_dirty(handle, inode);
+	}
+	ext4_free_blocks(handle, inode, new_bh, 0, 1,
+			 EXT4_FREE_BLOCKS_METADATA |
+			 EXT4_FREE_BLOCKS_FORGET);
+}
+
 /*
  * Find the available free space for EAs. This also returns the total number of
  * bytes used by EA entries.
@@ -2169,8 +2226,13 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 			ext4_xattr_block_cache_insert(ea_block_cache, new_bh);
 			error = ext4_handle_dirty_metadata(handle, inode,
 							   new_bh);
-			if (error)
+			if (error) {
+				ext4_xattr_new_block_fail(handle, inode, new_bh,
+							  i->in_inode ?
+							  i->value_len : 0,
+							  error);
 				goto cleanup;
+			}
 		}
 	}
 
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.