[PATCH] f2fs: check cp_error before committing atomic writes

[PATCH] f2fs: check cp_error before committing atomic writes

[Wenjie Qi]

F2FS checks cp_error before entering the ioctl path, but cp_error can be
set after that and before atomic commit starts replacing blocks.

In that case, F2FS_IOC_COMMIT_ATOMIC_WRITE returns -EIO, but the current
mount can still expose the new data before remount.

Check cp_error again in f2fs_commit_atomic_write() after
filemap_write_and_wait_range() and before block replacement starts.

Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
 fs/f2fs/segment.c | 3 +++
 1 file changed, 3 insertions(+)

QEMU validation:
- before fix, FAULT_CHECKPOINT made COMMIT_ATOMIC_WRITE fail with -EIO,
  while the current mount still exposed the new data; after remount, the
  file reverted to the old data.
- after fix, the same fault still failed with -EIO, and both the current
  mount and the remounted filesystem kept the old data.
- no-fault smoke still committed the new data successfully and preserved it
  across remount.

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 0b706568b034..e6384dd2e916 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -408,6 +408,9 @@ int f2fs_commit_atomic_write(struct inode *inode)
 	if (err)
 		return err;
 
+	if (unlikely(f2fs_cp_error(sbi)))
+		return -EIO;
+
 	f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
 	f2fs_lock_op(sbi, &lc);
 
-- 
2.43.0

Re: [PATCH] f2fs: check cp_error before committing atomic writes

[Chao Yu]

On 8/6/26 14:18, Wenjie Qi wrote:
> F2FS checks cp_error before entering the ioctl path, but cp_error can be
> set after that and before atomic commit starts replacing blocks.
> 
> In that case, F2FS_IOC_COMMIT_ATOMIC_WRITE returns -EIO, but the current
> mount can still expose the new data before remount.
> 
> Check cp_error again in f2fs_commit_atomic_write() after
> filemap_write_and_wait_range() and before block replacement starts.
> 
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
>  fs/f2fs/segment.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> QEMU validation:
> - before fix, FAULT_CHECKPOINT made COMMIT_ATOMIC_WRITE fail with -EIO,
>   while the current mount still exposed the new data; after remount, the
>   file reverted to the old data.
> - after fix, the same fault still failed with -EIO, and both the current
>   mount and the remounted filesystem kept the old data.
> - no-fault smoke still committed the new data successfully and preserved it
>   across remount.
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 0b706568b034..e6384dd2e916 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -408,6 +408,9 @@ int f2fs_commit_atomic_write(struct inode *inode)
>  	if (err)
>  		return err;
>  
> +	if (unlikely(f2fs_cp_error(sbi)))
> +		return -EIO;

I suspect even we add cp_error in more places, we may suffer both conditions:
1) expose new data to userspace if cp_error occurs before f2fs_do_sync_file()
after f2fs_commit_atomic_write().
2) not expose new data to userspace if we cp_error occurs before or during
__f2fs_commit_atomic_write().

Thanks,

> +
>  	f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>  	f2fs_lock_op(sbi, &lc);
>