[PATCH] f2fs: release a removed xattr node in roll-forward recovery

[PATCH] f2fs: release a removed xattr node in roll-forward recovery

[Daeho Jeong]

From: Daeho Jeong <daehojeong@google.com>

When xattrs shrink enough to fit in the inline xattr area,
write_all_xattrs() truncates the xattr node and clears i_xattr_nid. If
the file is then fsynced without a checkpoint, only the inode block is
written, with i_xattr_nid of 0.

Roll-forward recovery starts from the checkpointed inode, whose
i_xattr_nid still points to the old xattr node.
f2fs_recover_inline_xattr() copies only the inline xattr area, and
f2fs_update_inode() writes the old i_xattr_nid back. So the inode keeps
the stale xattr node:

 - The node block is never released, and fsck reports "nonzero bytes in
   xattr space after end of list".
 - If the inline xattr list ends exactly at the end of the inline area,
   its terminator is read from the stale node, and the removed xattrs
   come back after recovery.

When the fsynced inode has no xattr node but the checkpointed one has,
release the old xattr node in the same way as f2fs_recover_xattr_data().

Fixes: 28cdce0459cc ("f2fs: recover inline xattr data in roll-forward process")
Cc: stable@vger.kernel.org
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 fs/f2fs/node.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 55008f18afd..590fdedf9c2 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2831,6 +2831,29 @@ int f2fs_recover_inline_xattr(struct inode *inode, struct f2fs_cached_block *ent
 	f2fs_cache_wait_writeback(ientry);
 	memcpy(dst_addr, src_addr, inline_size);
 update_inode:
+	/*
+	 * The xattr node of the checkpoint was removed before fsync, e.g. the
+	 * xattrs became small enough to fit inline. Release it as well, or
+	 * the inode keeps pointing to the stale xattrs.
+	 */
+	if (!ri->i_xattr_nid && F2FS_I(inode)->i_xattr_nid) {
+		struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+		struct node_info ni;
+		int err;
+
+		err = f2fs_get_node_info(sbi, F2FS_I(inode)->i_xattr_nid,
+								&ni, false);
+		if (err) {
+			f2fs_put_cache(ientry, true);
+			return err;
+		}
+
+		f2fs_invalidate_blocks(sbi, ni.blk_addr, 1);
+		dec_valid_node_count(sbi, inode, false);
+		set_node_addr(sbi, &ni, NULL_ADDR, false);
+		f2fs_i_xnid_write(inode, 0);
+	}
+
 	f2fs_update_inode(inode, ientry);
 	f2fs_put_cache(ientry, true);
 	return 0;
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [f2fs-dev] [PATCH] f2fs: release a removed xattr node in roll-forward recovery

[Chao Yu]

On 10/13/26 10:09, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> When xattrs shrink enough to fit in the inline xattr area,
> write_all_xattrs() truncates the xattr node and clears i_xattr_nid. If
> the file is then fsynced without a checkpoint, only the inode block is
> written, with i_xattr_nid of 0.
> 
> Roll-forward recovery starts from the checkpointed inode, whose
> i_xattr_nid still points to the old xattr node.
> f2fs_recover_inline_xattr() copies only the inline xattr area, and
> f2fs_update_inode() writes the old i_xattr_nid back. So the inode keeps
> the stale xattr node:
> 
>  - The node block is never released, and fsck reports "nonzero bytes in
>    xattr space after end of list".
>  - If the inline xattr list ends exactly at the end of the inline area,
>    its terminator is read from the stale node, and the removed xattrs
>    come back after recovery.
> 
> When the fsynced inode has no xattr node but the checkpointed one has,
> release the old xattr node in the same way as f2fs_recover_xattr_data().
> 
> Fixes: 28cdce0459cc ("f2fs: recover inline xattr data in roll-forward process")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> ---
>  fs/f2fs/node.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 55008f18afd..590fdedf9c2 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2831,6 +2831,29 @@ int f2fs_recover_inline_xattr(struct inode *inode, struct f2fs_cached_block *ent
>  	f2fs_cache_wait_writeback(ientry);
>  	memcpy(dst_addr, src_addr, inline_size);
>  update_inode:
> +	/*
> +	 * The xattr node of the checkpoint was removed before fsync, e.g. the
> +	 * xattrs became small enough to fit inline. Release it as well, or
> +	 * the inode keeps pointing to the stale xattrs.
> +	 */
> +	if (!ri->i_xattr_nid && F2FS_I(inode)->i_xattr_nid) {
> +		struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> +		struct node_info ni;
> +		int err;
> +
> +		err = f2fs_get_node_info(sbi, F2FS_I(inode)->i_xattr_nid,
> +								&ni, false);
> +		if (err) {
> +			f2fs_put_cache(ientry, true);
> +			return err;
> +		}
> +
> +		f2fs_invalidate_blocks(sbi, ni.blk_addr, 1);
> +		dec_valid_node_count(sbi, inode, false);
> +		set_node_addr(sbi, &ni, NULL_ADDR, false);
> +		f2fs_i_xnid_write(inode, 0);

I think we need to clear dirty on xnode as well, refer to truncate_node(), otherwise
f2fs_writeback will write a dirty xnode w/ blkaddr = NULL_ADDR.

Thanks,

> +	}
> +
>  	f2fs_update_inode(inode, ientry);
>  	f2fs_put_cache(ientry, true);
>  	return 0;