ext4_xattr_block_set() acquires a reference to the ea_inode via ext4_xattr_inode_lookup_create() before constructing the new xattr block. In the new-block allocation path this extra reference is dropped after ext4_xattr_inode_inc_ref_all() accounts for every entry in the block. But when mbcache supplies an identical existing block and that block is reused, the cached block already carries the correct refcount for its entries, and the extra reference is never dropped. The cleanup block at the end of the function only calls ext4_xattr_inode_dec_ref() when error is non-zero, so the success path through cache reuse leaks one refcount. Each reuse event adds another; eventually the saturated refcount prevents the ea_inode from being freed when its last real reference is dropped. Add the matching drop when the entry was not written directly to the old physical buffer (bs->bh && s->base == bs->bh->b_data). In that case the reference belongs to the old buffer, which the later release_block() handles. When s->base is a clone or a fresh allocation and is discarded in favour of the cached block, the extra reference must be dropped. v2: - The v1 dropped the reference best-effort: a failure was logged but masked by the later "error = 0" on the success path, so the syscall could succeed with the on-disk refcount one too high. Fail the operation instead, and undo the locally-owned part of the reuse through ext4_xattr_reuse_undo(), the same helper the success path uses for the old block. Any reference that nonetheless persists (e.g. under errors=continue) is repaired by e2fsck. - A failed decrement is never retried: it may already have mutated the refcount before failing (e.g. an EA inode inheriting S_SYNC from the root directory can fail ext4_mark_iloc_dirty() after the mutation in nojournal mode), so a retry could decrement twice. The failure paths free the quota charge and drop the inode reference directly. - Propagate the same masked failure at the new-block branch's identical drop: it has always been best-effort (warn and continue). Found by the Sashiko AI review bot while reviewing the xattr saturation-livelock fix. Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock") Signed-off-by: Matthias Goergens --- fs/ext4/xattr.c | 62 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 982a1f831e228..0ce937d890042 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -1362,6 +1362,17 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode, return; } +/* Undo the locally-owned part of a cached-block reuse. */ +static void ext4_xattr_reuse_undo(handle_t *handle, struct inode *inode, + struct buffer_head *bh) +{ + struct ext4_xattr_inode_array *ea_inode_array = NULL; + + ext4_xattr_release_block(handle, inode, bh, &ea_inode_array, + 0 /* extra_credits */); + ext4_xattr_inode_array_free(ea_inode_array); +} + /* * Find the available free space for EAs. This also returns the total number of * bytes used by EA entries. @@ -2107,6 +2118,39 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode, mb_cache_entry_touch(ea_block_cache, ce); mb_cache_entry_put(ea_block_cache, ce); ce = NULL; + if (!(bs->bh && s->base == bs->bh->b_data) && + ea_inode) { + /* + * The reused block already holds its own + * reference; drop the extra one unless the + * entry went into the old buffer directly. + */ + error = ext4_xattr_inode_dec_ref(handle, + ea_inode); + if (error) { + /* + * The decrement may already have run; + * do not retry it: free the charge, + * undo the reuse, and fail the op. + */ + ext4_error_inode(inode, __func__, + __LINE__, 0, + "dec ref error=%d", + error); + ext4_xattr_inode_free_quota(inode, + ea_inode, + i_size_read(ea_inode)); + iput(ea_inode); + ea_inode = NULL; + if (new_bh != bs->bh) + ext4_xattr_reuse_undo(handle, + inode, + new_bh); + goto cleanup; + } + iput(ea_inode); + ea_inode = NULL; + } } else if (bs->bh && s->base == bs->bh->b_data) { /* We were modifying this block in-place. */ ea_bdebug(bs->bh, "keeping this block"); @@ -2143,13 +2187,21 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode, if (error) goto getblk_failed; if (ea_inode) { - /* Drop the extra ref on ea_inode. */ + /* + * Drop the extra ref on ea_inode; on + * failure the decrement may already have + * run, so just fail and free the block. + */ error = ext4_xattr_inode_dec_ref(handle, ea_inode); - if (error) - ext4_warning_inode(ea_inode, - "dec ref error=%d", - error); + if (error) { + ext4_xattr_inode_free_quota(inode, + ea_inode, + i_size_read(ea_inode)); + iput(ea_inode); + ea_inode = NULL; + goto getblk_failed; + } iput(ea_inode); ea_inode = NULL; } -- 2.55.0