In f2fs_update_inode, i_size of the atomic file isn't updated until
FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
after the writeback of the inode, i_size of the raw inode will not be
updated. It can cause the atomicity corruption due to a mismatch between
old file size and new data.
To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
Atomic write thread Writeback thread
__writeback_single_inode
write_inode
f2fs_update_inode
- skip i_size update
f2fs_ioc_commit_atomic_write
f2fs_commit_atomic_write
set_inode_flag(inode, FI_ATOMIC_COMMITTED)
f2fs_do_sync_file
f2fs_fsync_node_pages
- skip f2fs_update_inode since the inode is clean
Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Cc: [email protected] #v5.19+
Reviewed-by: Sungjong Seo <[email protected]>
Reviewed-by: Yeongjin Gil <[email protected]>
Signed-off-by: Sunmin Jeong <[email protected]>
---
fs/f2fs/f2fs.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 543898482f8b..a000cb024dbe 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3039,6 +3039,7 @@ static inline void __mark_inode_dirty_flag(struct inode *inode,
case FI_INLINE_DOTS:
case FI_PIN_FILE:
case FI_COMPRESS_RELEASED:
+ case FI_ATOMIC_COMMITTED:
f2fs_mark_inode_dirty_sync(inode, true);
}
}
--
2.25.1
In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
between the original inode and COW inode. When aborting atomic write and
writeback occur simultaneously, invalid data can be written to original
inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
To prevent the problem, let's truncate all pages before clearing the flag
Atomic write thread Writeback thread
f2fs_abort_atomic_write
clear_inode_flag(inode, FI_ATOMIC_FILE)
__writeback_single_inode
do_writepages
f2fs_do_write_data_page
- use dn of original inode
truncate_inode_pages_final
Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Cc: [email protected] #v5.19+
Reviewed-by: Sungjong Seo <[email protected]>
Reviewed-by: Yeongjin Gil <[email protected]>
Signed-off-by: Sunmin Jeong <[email protected]>
---
fs/f2fs/segment.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 7901ede58113..7e47b8054413 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -192,6 +192,9 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
if (!f2fs_is_atomic_file(inode))
return;
+ if (clean)
+ truncate_inode_pages_final(inode->i_mapping);
+
release_atomic_write_cnt(inode);
clear_inode_flag(inode, FI_ATOMIC_COMMITTED);
clear_inode_flag(inode, FI_ATOMIC_REPLACE);
@@ -201,7 +204,6 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
F2FS_I(inode)->atomic_write_task = NULL;
if (clean) {
- truncate_inode_pages_final(inode->i_mapping);
f2fs_i_size_write(inode, fi->original_i_size);
fi->original_i_size = 0;
}
--
2.25.1
Reviewed-by: Daeho Jeong <[email protected]>
On Wed, Mar 13, 2024 at 4:37 AM Sunmin Jeong <[email protected]> wrote:
>
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> Atomic write thread Writeback thread
> __writeback_single_inode
> write_inode
> f2fs_update_inode
> - skip i_size update
> f2fs_ioc_commit_atomic_write
> f2fs_commit_atomic_write
> set_inode_flag(inode, FI_ATOMIC_COMMITTED)
> f2fs_do_sync_file
> f2fs_fsync_node_pages
> - skip f2fs_update_inode since the inode is clean
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: [email protected] #v5.19+
> Reviewed-by: Sungjong Seo <[email protected]>
> Reviewed-by: Yeongjin Gil <[email protected]>
> Signed-off-by: Sunmin Jeong <[email protected]>
> ---
> fs/f2fs/f2fs.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 543898482f8b..a000cb024dbe 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3039,6 +3039,7 @@ static inline void __mark_inode_dirty_flag(struct inode *inode,
> case FI_INLINE_DOTS:
> case FI_PIN_FILE:
> case FI_COMPRESS_RELEASED:
> + case FI_ATOMIC_COMMITTED:
> f2fs_mark_inode_dirty_sync(inode, true);
> }
> }
> --
> 2.25.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Reviewed-by: Daeho Jeong <[email protected]>
On Wed, Mar 13, 2024 at 4:29 AM Sunmin Jeong <[email protected]> wrote:
>
> In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
> between the original inode and COW inode. When aborting atomic write and
> writeback occur simultaneously, invalid data can be written to original
> inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
>
> To prevent the problem, let's truncate all pages before clearing the flag
>
> Atomic write thread Writeback thread
> f2fs_abort_atomic_write
> clear_inode_flag(inode, FI_ATOMIC_FILE)
> __writeback_single_inode
> do_writepages
> f2fs_do_write_data_page
> - use dn of original inode
> truncate_inode_pages_final
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: [email protected] #v5.19+
> Reviewed-by: Sungjong Seo <[email protected]>
> Reviewed-by: Yeongjin Gil <[email protected]>
> Signed-off-by: Sunmin Jeong <[email protected]>
> ---
> fs/f2fs/segment.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 7901ede58113..7e47b8054413 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -192,6 +192,9 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
> if (!f2fs_is_atomic_file(inode))
> return;
>
> + if (clean)
> + truncate_inode_pages_final(inode->i_mapping);
> +
> release_atomic_write_cnt(inode);
> clear_inode_flag(inode, FI_ATOMIC_COMMITTED);
> clear_inode_flag(inode, FI_ATOMIC_REPLACE);
> @@ -201,7 +204,6 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
> F2FS_I(inode)->atomic_write_task = NULL;
>
> if (clean) {
> - truncate_inode_pages_final(inode->i_mapping);
> f2fs_i_size_write(inode, fi->original_i_size);
> fi->original_i_size = 0;
> }
> --
> 2.25.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 2024/3/13 19:26, Sunmin Jeong wrote:
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> Atomic write thread Writeback thread
> __writeback_single_inode
> write_inode
> f2fs_update_inode
> - skip i_size update
> f2fs_ioc_commit_atomic_write
> f2fs_commit_atomic_write
> set_inode_flag(inode, FI_ATOMIC_COMMITTED)
> f2fs_do_sync_file
> f2fs_fsync_node_pages
> - skip f2fs_update_inode since the inode is clean
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: [email protected] #v5.19+
> Reviewed-by: Sungjong Seo <[email protected]>
> Reviewed-by: Yeongjin Gil <[email protected]>
> Signed-off-by: Sunmin Jeong <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Thanks,
On 2024/3/13 19:26, Sunmin Jeong wrote:
> In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
> between the original inode and COW inode. When aborting atomic write and
> writeback occur simultaneously, invalid data can be written to original
> inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
>
> To prevent the problem, let's truncate all pages before clearing the flag
>
> Atomic write thread Writeback thread
> f2fs_abort_atomic_write
> clear_inode_flag(inode, FI_ATOMIC_FILE)
> __writeback_single_inode
> do_writepages
> f2fs_do_write_data_page
> - use dn of original inode
> truncate_inode_pages_final
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: [email protected] #v5.19+
> Reviewed-by: Sungjong Seo <[email protected]>
> Reviewed-by: Yeongjin Gil <[email protected]>
> Signed-off-by: Sunmin Jeong <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Thanks,
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[email protected]>:
On Wed, 13 Mar 2024 20:26:19 +0900 you wrote:
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> [...]
Here is the summary with links:
- [f2fs-dev,1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
https://git.kernel.org/jaegeuk/f2fs/c/4bf78322346f
- [f2fs-dev,2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
https://git.kernel.org/jaegeuk/f2fs/c/74b0ebcbdde4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html