[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