Linux EXT4 FS development
 help / color / mirror / Atom feed
From: Matthias Goergens <matthias.goergens@gmail.com>
To: linux-ext4@vger.kernel.org
Cc: tytso@mit.edu, jack@suse.cz
Subject: [PATCH] ext4: compensate ea_inode refs and free block on new xattr block write error
Date: Mon, 17 Aug 2026 17:54:21 +0800	[thread overview]
Message-ID: <20260817095421.1171145-1-matthias.goergens@gmail.com> (raw)
In-Reply-To: <20260814070432.595B91F00A3A@smtp.kernel.org>

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 <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-ext4/20260814070432.595B91F00A3A@smtp.kernel.org/
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
 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


  reply	other threads:[~2026-08-17  9:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12  4:45 [PATCH 1/1] ext4: drop extra ea_inode ref in cached-block reuse path Matthias Goergens
2026-08-12  5:05 ` sashiko-bot
2026-08-14  6:50 ` [PATCH v2] " Matthias Goergens
2026-08-14  7:04   ` sashiko-bot
2026-08-17  9:54     ` Matthias Goergens [this message]
2026-08-17 10:07       ` [PATCH] ext4: compensate ea_inode refs and free block on new xattr block write error sashiko-bot
2026-08-17 11:49         ` Matthias Goergens
2026-08-17 15:29           ` [PATCH] ext4: compensate ea_inode refs and free block on new xattr block write error\ Theodore Tso
2026-08-18 14:09   ` [PATCH v2] ext4: drop extra ea_inode ref in cached-block reuse path Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260817095421.1171145-1-matthias.goergens@gmail.com \
    --to=matthias.goergens@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox