* [PATCH v2] f2fs: fix to zero post-eof page
@ 2025-06-04 8:57 Chao Yu
2025-06-05 1:15 ` [f2fs-dev] " Zhiguo Niu
0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2025-06-04 8:57 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
fstest reports a f2fs bug:
generic/363 42s ... [failed, exit status 1]- output mismatch (see /share/git/fstests/results//generic/363.out.bad)
--- tests/generic/363.out 2025-01-12 21:57:40.271440542 +0800
+++ /share/git/fstests/results//generic/363.out.bad 2025-05-19 19:55:58.000000000 +0800
@@ -1,2 +1,78 @@
QA output created by 363
fsx -q -S 0 -e 1 -N 100000
+READ BAD DATA: offset = 0xd6fb, size = 0xf044, fname = /mnt/f2fs/junk
+OFFSET GOOD BAD RANGE
+0x1540d 0x0000 0x2a25 0x0
+operation# (mod 256) for the bad data may be 37
+0x1540e 0x0000 0x2527 0x1
...
(Run 'diff -u /share/git/fstests/tests/generic/363.out /share/git/fstests/results//generic/363.out.bad' to see the entire diff)
Ran: generic/363
Failures: generic/363
Failed 1 of 1 tests
The root cause is user can update post-eof page via mmap, however, f2fs missed
to zero post-eof page in below operations, so, once it expands i_size, then it
will include dummy data locates previous post-eof page, so during below
operations, we need to zero post-eof page.
Operations which can include dummy data after previous i_size after expanding
i_size:
- write
- mapwrite
- truncate
- fallocate
* preallocate
* zero_range
* insert_range
* collapse_range
- clone_range (doesn’t support in f2fs)
- copy_range (doesn’t support in f2fs)
Cc: stable@kernel.org
Signed-off-by: Chao Yu <chao@kernel.org>
---
v2:
- cover f2fs_zero_post_eof_page w/ invalidate_lock
fs/f2fs/file.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 6bd3de64f2a8..ee5e662d2a4c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -35,6 +35,17 @@
#include <trace/events/f2fs.h>
#include <uapi/linux/f2fs.h>
+static void f2fs_zero_post_eof_page(struct inode *inode, loff_t new_size)
+{
+ loff_t old_size = i_size_read(inode);
+
+ if (old_size > new_size)
+ return;
+
+ /* zero or drop pages only in range of [old_size, new_size] */
+ truncate_pagecache(inode, old_size);
+}
+
static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
{
struct inode *inode = file_inode(vmf->vma->vm_file);
@@ -103,8 +114,13 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
+ filemap_invalidate_lock(inode->i_mapping);
+ f2fs_zero_post_eof_page(inode, (folio->index + 1) << PAGE_SHIFT);
+ filemap_invalidate_unlock(inode->i_mapping);
+
file_update_time(vmf->vma->vm_file);
filemap_invalidate_lock_shared(inode->i_mapping);
+
folio_lock(folio);
if (unlikely(folio->mapping != inode->i_mapping ||
folio_pos(folio) > i_size_read(inode) ||
@@ -1109,6 +1125,8 @@ int f2fs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
filemap_invalidate_lock(inode->i_mapping);
+ if (attr->ia_size > old_size)
+ f2fs_zero_post_eof_page(inode, old_size);
truncate_setsize(inode, attr->ia_size);
if (attr->ia_size <= old_size)
@@ -1227,6 +1245,10 @@ static int f2fs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
if (ret)
return ret;
+ filemap_invalidate_lock(inode->i_mapping);
+ f2fs_zero_post_eof_page(inode, offset + len);
+ filemap_invalidate_unlock(inode->i_mapping);
+
pg_start = ((unsigned long long) offset) >> PAGE_SHIFT;
pg_end = ((unsigned long long) offset + len) >> PAGE_SHIFT;
@@ -1510,6 +1532,8 @@ static int f2fs_do_collapse(struct inode *inode, loff_t offset, loff_t len)
f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
filemap_invalidate_lock(inode->i_mapping);
+ f2fs_zero_post_eof_page(inode, offset + len);
+
f2fs_lock_op(sbi);
f2fs_drop_extent_tree(inode);
truncate_pagecache(inode, offset);
@@ -1631,6 +1655,10 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
if (ret)
return ret;
+ filemap_invalidate_lock(mapping);
+ f2fs_zero_post_eof_page(inode, offset + len);
+ filemap_invalidate_unlock(mapping);
+
pg_start = ((unsigned long long) offset) >> PAGE_SHIFT;
pg_end = ((unsigned long long) offset + len) >> PAGE_SHIFT;
@@ -1762,6 +1790,8 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
/* avoid gc operation during block exchange */
f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
filemap_invalidate_lock(mapping);
+
+ f2fs_zero_post_eof_page(inode, offset + len);
truncate_pagecache(inode, offset);
while (!ret && idx > pg_start) {
@@ -1819,6 +1849,10 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
if (err)
return err;
+ filemap_invalidate_lock(inode->i_mapping);
+ f2fs_zero_post_eof_page(inode, offset + len);
+ filemap_invalidate_unlock(inode->i_mapping);
+
f2fs_balance_fs(sbi, true);
pg_start = ((unsigned long long)offset) >> PAGE_SHIFT;
@@ -4860,6 +4894,10 @@ static ssize_t f2fs_write_checks(struct kiocb *iocb, struct iov_iter *from)
err = file_modified(file);
if (err)
return err;
+
+ filemap_invalidate_lock(inode->i_mapping);
+ f2fs_zero_post_eof_page(inode, iocb->ki_pos + iov_iter_count(from));
+ filemap_invalidate_unlock(inode->i_mapping);
return count;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix to zero post-eof page
2025-06-04 8:57 [PATCH v2] f2fs: fix to zero post-eof page Chao Yu
@ 2025-06-05 1:15 ` Zhiguo Niu
2025-06-05 3:16 ` Chao Yu
0 siblings, 1 reply; 3+ messages in thread
From: Zhiguo Niu @ 2025-06-05 1:15 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, stable, linux-kernel, linux-f2fs-devel
Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
于2025年6月4日周三 17:01写道：
>
> fstest reports a f2fs bug:
>
> generic/363 42s ... [failed, exit status 1]- output mismatch (see /share/git/fstests/results//generic/363.out.bad)
> --- tests/generic/363.out 2025-01-12 21:57:40.271440542 +0800
> +++ /share/git/fstests/results//generic/363.out.bad 2025-05-19 19:55:58.000000000 +0800
> @@ -1,2 +1,78 @@
> QA output created by 363
> fsx -q -S 0 -e 1 -N 100000
> +READ BAD DATA: offset = 0xd6fb, size = 0xf044, fname = /mnt/f2fs/junk
> +OFFSET GOOD BAD RANGE
> +0x1540d 0x0000 0x2a25 0x0
> +operation# (mod 256) for the bad data may be 37
> +0x1540e 0x0000 0x2527 0x1
> ...
> (Run 'diff -u /share/git/fstests/tests/generic/363.out /share/git/fstests/results//generic/363.out.bad' to see the entire diff)
> Ran: generic/363
> Failures: generic/363
> Failed 1 of 1 tests
>
> The root cause is user can update post-eof page via mmap, however, f2fs missed
> to zero post-eof page in below operations, so, once it expands i_size, then it
> will include dummy data locates previous post-eof page, so during below
> operations, we need to zero post-eof page.
>
> Operations which can include dummy data after previous i_size after expanding
> i_size:
> - write
> - mapwrite
> - truncate
> - fallocate
> * preallocate
> * zero_range
> * insert_range
> * collapse_range
> - clone_range (doesn’t support in f2fs)
> - copy_range (doesn’t support in f2fs)
>
> Cc: stable@kernel.org
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> v2:
> - cover f2fs_zero_post_eof_page w/ invalidate_lock
> fs/f2fs/file.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6bd3de64f2a8..ee5e662d2a4c 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -35,6 +35,17 @@
> #include <trace/events/f2fs.h>
> #include <uapi/linux/f2fs.h>
>
> +static void f2fs_zero_post_eof_page(struct inode *inode, loff_t new_size)
> +{
> + loff_t old_size = i_size_read(inode);
> +
> + if (old_size > new_size)
Hi Chao,
should it be old_size >= new_size?
> + return;
> +
> + /* zero or drop pages only in range of [old_size, new_size] */
> + truncate_pagecache(inode, old_size);
> +}
> +
> static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
> {
> struct inode *inode = file_inode(vmf->vma->vm_file);
> @@ -103,8 +114,13 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
>
> f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
>
> + filemap_invalidate_lock(inode->i_mapping);
> + f2fs_zero_post_eof_page(inode, (folio->index + 1) << PAGE_SHIFT);
> + filemap_invalidate_unlock(inode->i_mapping);
> +
> file_update_time(vmf->vma->vm_file);
> filemap_invalidate_lock_shared(inode->i_mapping);
> +
> folio_lock(folio);
> if (unlikely(folio->mapping != inode->i_mapping ||
> folio_pos(folio) > i_size_read(inode) ||
> @@ -1109,6 +1125,8 @@ int f2fs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
> filemap_invalidate_lock(inode->i_mapping);
>
> + if (attr->ia_size > old_size)
> + f2fs_zero_post_eof_page(inode, old_size);
I'm a little suspicious about the logic here， here old_size is from
i_size_read(inode),
and it will compare with the "old_size" in f2fs_zero_post_eof_page, it
is also from i_size_read(inode),
so is this actually meaningless?
Thanks!
> truncate_setsize(inode, attr->ia_size);
>
> if (attr->ia_size <= old_size)
> @@ -1227,6 +1245,10 @@ static int f2fs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
> if (ret)
> return ret;
>
> + filemap_invalidate_lock(inode->i_mapping);
> + f2fs_zero_post_eof_page(inode, offset + len);
> + filemap_invalidate_unlock(inode->i_mapping);
> +
> pg_start = ((unsigned long long) offset) >> PAGE_SHIFT;
> pg_end = ((unsigned long long) offset + len) >> PAGE_SHIFT;
>
> @@ -1510,6 +1532,8 @@ static int f2fs_do_collapse(struct inode *inode, loff_t offset, loff_t len)
> f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> filemap_invalidate_lock(inode->i_mapping);
>
> + f2fs_zero_post_eof_page(inode, offset + len);
> +
> f2fs_lock_op(sbi);
> f2fs_drop_extent_tree(inode);
> truncate_pagecache(inode, offset);
> @@ -1631,6 +1655,10 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
> if (ret)
> return ret;
>
> + filemap_invalidate_lock(mapping);
> + f2fs_zero_post_eof_page(inode, offset + len);
> + filemap_invalidate_unlock(mapping);
> +
> pg_start = ((unsigned long long) offset) >> PAGE_SHIFT;
> pg_end = ((unsigned long long) offset + len) >> PAGE_SHIFT;
>
> @@ -1762,6 +1790,8 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
> /* avoid gc operation during block exchange */
> f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> filemap_invalidate_lock(mapping);
> +
> + f2fs_zero_post_eof_page(inode, offset + len);
> truncate_pagecache(inode, offset);
>
> while (!ret && idx > pg_start) {
> @@ -1819,6 +1849,10 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
> if (err)
> return err;
>
> + filemap_invalidate_lock(inode->i_mapping);
> + f2fs_zero_post_eof_page(inode, offset + len);
> + filemap_invalidate_unlock(inode->i_mapping);
> +
> f2fs_balance_fs(sbi, true);
>
> pg_start = ((unsigned long long)offset) >> PAGE_SHIFT;
> @@ -4860,6 +4894,10 @@ static ssize_t f2fs_write_checks(struct kiocb *iocb, struct iov_iter *from)
> err = file_modified(file);
> if (err)
> return err;
> +
> + filemap_invalidate_lock(inode->i_mapping);
> + f2fs_zero_post_eof_page(inode, iocb->ki_pos + iov_iter_count(from));
> + filemap_invalidate_unlock(inode->i_mapping);
> return count;
> }
>
> --
> 2.49.0
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix to zero post-eof page
2025-06-05 1:15 ` [f2fs-dev] " Zhiguo Niu
@ 2025-06-05 3:16 ` Chao Yu
0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2025-06-05 3:16 UTC (permalink / raw)
To: Zhiguo Niu; +Cc: chao, jaegeuk, stable, linux-kernel, linux-f2fs-devel
On 6/5/25 09:15, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> 于2025年6月4日周三 17:01写道：
>>
>> fstest reports a f2fs bug:
>>
>> generic/363 42s ... [failed, exit status 1]- output mismatch (see /share/git/fstests/results//generic/363.out.bad)
>> --- tests/generic/363.out 2025-01-12 21:57:40.271440542 +0800
>> +++ /share/git/fstests/results//generic/363.out.bad 2025-05-19 19:55:58.000000000 +0800
>> @@ -1,2 +1,78 @@
>> QA output created by 363
>> fsx -q -S 0 -e 1 -N 100000
>> +READ BAD DATA: offset = 0xd6fb, size = 0xf044, fname = /mnt/f2fs/junk
>> +OFFSET GOOD BAD RANGE
>> +0x1540d 0x0000 0x2a25 0x0
>> +operation# (mod 256) for the bad data may be 37
>> +0x1540e 0x0000 0x2527 0x1
>> ...
>> (Run 'diff -u /share/git/fstests/tests/generic/363.out /share/git/fstests/results//generic/363.out.bad' to see the entire diff)
>> Ran: generic/363
>> Failures: generic/363
>> Failed 1 of 1 tests
>>
>> The root cause is user can update post-eof page via mmap, however, f2fs missed
>> to zero post-eof page in below operations, so, once it expands i_size, then it
>> will include dummy data locates previous post-eof page, so during below
>> operations, we need to zero post-eof page.
>>
>> Operations which can include dummy data after previous i_size after expanding
>> i_size:
>> - write
>> - mapwrite
>> - truncate
>> - fallocate
>> * preallocate
>> * zero_range
>> * insert_range
>> * collapse_range
>> - clone_range (doesn’t support in f2fs)
>> - copy_range (doesn’t support in f2fs)
>>
>> Cc: stable@kernel.org
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> v2:
>> - cover f2fs_zero_post_eof_page w/ invalidate_lock
>> fs/f2fs/file.c | 38 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 6bd3de64f2a8..ee5e662d2a4c 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -35,6 +35,17 @@
>> #include <trace/events/f2fs.h>
>> #include <uapi/linux/f2fs.h>
>>
>> +static void f2fs_zero_post_eof_page(struct inode *inode, loff_t new_size)
>> +{
>> + loff_t old_size = i_size_read(inode);
>> +
>> + if (old_size > new_size)
> Hi Chao,
> should it be old_size >= new_size?
Zhiguo,
Yes, you're right.
>
>> + return;
>> +
>> + /* zero or drop pages only in range of [old_size, new_size] */
>> + truncate_pagecache(inode, old_size);
>> +}
>> +
>> static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
>> {
>> struct inode *inode = file_inode(vmf->vma->vm_file);
>> @@ -103,8 +114,13 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
>>
>> f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
>>
>> + filemap_invalidate_lock(inode->i_mapping);
>> + f2fs_zero_post_eof_page(inode, (folio->index + 1) << PAGE_SHIFT);
>> + filemap_invalidate_unlock(inode->i_mapping);
>> +
>> file_update_time(vmf->vma->vm_file);
>> filemap_invalidate_lock_shared(inode->i_mapping);
>> +
>> folio_lock(folio);
>> if (unlikely(folio->mapping != inode->i_mapping ||
>> folio_pos(folio) > i_size_read(inode) ||
>> @@ -1109,6 +1125,8 @@ int f2fs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>> f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>> filemap_invalidate_lock(inode->i_mapping);
>>
>> + if (attr->ia_size > old_size)
>> + f2fs_zero_post_eof_page(inode, old_size);
> I'm a little suspicious about the logic here， here old_size is from
> i_size_read(inode),
> and it will compare with the "old_size" in f2fs_zero_post_eof_page, it
> is also from i_size_read(inode),
> so is this actually meaningless?
Oh, I need to pass attr->ia_size instead of old_size.
Let me revise in v3, anyway, thanks for the review.
Thanks,
> Thanks!
>> truncate_setsize(inode, attr->ia_size);
>>
>> if (attr->ia_size <= old_size)
>> @@ -1227,6 +1245,10 @@ static int f2fs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
>> if (ret)
>> return ret;
>>
>> + filemap_invalidate_lock(inode->i_mapping);
>> + f2fs_zero_post_eof_page(inode, offset + len);
>> + filemap_invalidate_unlock(inode->i_mapping);
>> +
>> pg_start = ((unsigned long long) offset) >> PAGE_SHIFT;
>> pg_end = ((unsigned long long) offset + len) >> PAGE_SHIFT;
>>
>> @@ -1510,6 +1532,8 @@ static int f2fs_do_collapse(struct inode *inode, loff_t offset, loff_t len)
>> f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> filemap_invalidate_lock(inode->i_mapping);
>>
>> + f2fs_zero_post_eof_page(inode, offset + len);
>> +
>> f2fs_lock_op(sbi);
>> f2fs_drop_extent_tree(inode);
>> truncate_pagecache(inode, offset);
>> @@ -1631,6 +1655,10 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
>> if (ret)
>> return ret;
>>
>> + filemap_invalidate_lock(mapping);
>> + f2fs_zero_post_eof_page(inode, offset + len);
>> + filemap_invalidate_unlock(mapping);
>> +
>> pg_start = ((unsigned long long) offset) >> PAGE_SHIFT;
>> pg_end = ((unsigned long long) offset + len) >> PAGE_SHIFT;
>>
>> @@ -1762,6 +1790,8 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
>> /* avoid gc operation during block exchange */
>> f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> filemap_invalidate_lock(mapping);
>> +
>> + f2fs_zero_post_eof_page(inode, offset + len);
>> truncate_pagecache(inode, offset);
>>
>> while (!ret && idx > pg_start) {
>> @@ -1819,6 +1849,10 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
>> if (err)
>> return err;
>>
>> + filemap_invalidate_lock(inode->i_mapping);
>> + f2fs_zero_post_eof_page(inode, offset + len);
>> + filemap_invalidate_unlock(inode->i_mapping);
>> +
>> f2fs_balance_fs(sbi, true);
>>
>> pg_start = ((unsigned long long)offset) >> PAGE_SHIFT;
>> @@ -4860,6 +4894,10 @@ static ssize_t f2fs_write_checks(struct kiocb *iocb, struct iov_iter *from)
>> err = file_modified(file);
>> if (err)
>> return err;
>> +
>> + filemap_invalidate_lock(inode->i_mapping);
>> + f2fs_zero_post_eof_page(inode, iocb->ki_pos + iov_iter_count(from));
>> + filemap_invalidate_unlock(inode->i_mapping);
>> return count;
>> }
>>
>> --
>> 2.49.0
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-05 3:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-04 8:57 [PATCH v2] f2fs: fix to zero post-eof page Chao Yu
2025-06-05 1:15 ` [f2fs-dev] " Zhiguo Niu
2025-06-05 3:16 ` Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).