[f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[Dmitry Antipov]

In 'f2fs_write_end_io()', end all of the in-progress writes with
-EFSCORRUPTED if the corresponding error was detected earlier.

Reported-by: syzbot+803dd716c4310d16ff3a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=803dd716c4310d16ff3a
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 fs/f2fs/data.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 54f89f0ee69b..755806868f0e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -320,9 +320,11 @@ static void f2fs_write_end_io(struct bio *bio)
 {
 	struct f2fs_sb_info *sbi;
 	struct folio_iter fi;
+	bool check;
 
 	iostat_update_and_unbind_ctx(bio);
 	sbi = bio->bi_private;
+	check = is_sbi_flag_set(sbi, SBI_NEED_FSCK);
 
 	if (time_to_inject(sbi, FAULT_WRITE_IO))
 		bio->bi_status = BLK_STS_IOERR;
@@ -331,6 +333,11 @@ static void f2fs_write_end_io(struct bio *bio)
 		struct folio *folio = fi.folio;
 		enum count_type type;
 
+		if (check) {
+			mapping_set_error(folio->mapping, -EFSCORRUPTED);
+			continue;
+		}
+
 		if (fscrypt_is_bounce_folio(folio)) {
 			struct folio *io_folio = folio;
 
-- 
2.49.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[Chao Yu via Linux-f2fs-devel]

On 4/24/25 00:15, Dmitry Antipov wrote:
> In 'f2fs_write_end_io()', end all of the in-progress writes with
> -EFSCORRUPTED if the corresponding error was detected earlier.

I guess we need to figure out why f2fs_bug_on() will be tiggerred?

f2fs_write_end_io()
{
...
		f2fs_bug_on(sbi, folio->mapping == NODE_MAPPING(sbi) &&
				folio->index != nid_of_node(&folio->page));

Thanks,

> 
> Reported-by: syzbot+803dd716c4310d16ff3a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=803dd716c4310d16ff3a
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  fs/f2fs/data.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 54f89f0ee69b..755806868f0e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -320,9 +320,11 @@ static void f2fs_write_end_io(struct bio *bio)
>  {
>  	struct f2fs_sb_info *sbi;
>  	struct folio_iter fi;
> +	bool check;
>  
>  	iostat_update_and_unbind_ctx(bio);
>  	sbi = bio->bi_private;
> +	check = is_sbi_flag_set(sbi, SBI_NEED_FSCK);
>  
>  	if (time_to_inject(sbi, FAULT_WRITE_IO))
>  		bio->bi_status = BLK_STS_IOERR;
> @@ -331,6 +333,11 @@ static void f2fs_write_end_io(struct bio *bio)
>  		struct folio *folio = fi.folio;
>  		enum count_type type;
>  
> +		if (check) {
> +			mapping_set_error(folio->mapping, -EFSCORRUPTED);
> +			continue;
> +		}
> +
>  		if (fscrypt_is_bounce_folio(folio)) {
>  			struct folio *io_folio = folio;
>  



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[Dmitry Antipov]

On 4/24/25 11:58 AM, Chao Yu wrote:

> I guess we need to figure out why f2fs_bug_on() will be tiggerred?
> 
> f2fs_write_end_io()
> {
> ...
> 		f2fs_bug_on(sbi, folio->mapping == NODE_MAPPING(sbi) &&
> 				folio->index != nid_of_node(&folio->page));

Well, syzkaller is very "creative" in generating weirdly damaged filesystems :-).

An overall idea of the patch is that an attempt to complete I/O on a filesystem
which is known to be "broken enough to run fsck" may make the things even worse
(by making the filesystem even less consistent). So it may be safer just to
refuse all in-flight write attempts at least.

Dmitry



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[Chao Yu via Linux-f2fs-devel]

On 4/24/25 00:15, Dmitry Antipov wrote:
> In 'f2fs_write_end_io()', end all of the in-progress writes with
> -EFSCORRUPTED if the corresponding error was detected earlier.
> 
> Reported-by: syzbot+803dd716c4310d16ff3a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=803dd716c4310d16ff3a
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  fs/f2fs/data.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 54f89f0ee69b..755806868f0e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -320,9 +320,11 @@ static void f2fs_write_end_io(struct bio *bio)
>  {
>  	struct f2fs_sb_info *sbi;
>  	struct folio_iter fi;
> +	bool check;
>  
>  	iostat_update_and_unbind_ctx(bio);
>  	sbi = bio->bi_private;
> +	check = is_sbi_flag_set(sbi, SBI_NEED_FSCK);
>  
>  	if (time_to_inject(sbi, FAULT_WRITE_IO))
>  		bio->bi_status = BLK_STS_IOERR;
> @@ -331,6 +333,11 @@ static void f2fs_write_end_io(struct bio *bio)
>  		struct folio *folio = fi.folio;
>  		enum count_type type;
>  
> +		if (check) {
> +			mapping_set_error(folio->mapping, -EFSCORRUPTED);
> +			continue;

Backing to the implementation, it seems it won't release resources
below, e.g. missed to call:

- fscrypt_free_bounce_page()
- f2fs_compress_write_end_io()
...
- folio_end_writeback()

I guess it may hang kernel w/ this patch? Or any thing I missed?

Thanks,

> +		}
> +
>  		if (fscrypt_is_bounce_folio(folio)) {
>  			struct folio *io_folio = folio;
>  



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[Chao Yu via Linux-f2fs-devel]

On 4/24/25 18:43, Dmitry Antipov wrote:
> On 4/24/25 11:58 AM, Chao Yu wrote:
> 
>> I guess we need to figure out why f2fs_bug_on() will be tiggerred?
>>
>> f2fs_write_end_io()
>> {
>> ...
>>         f2fs_bug_on(sbi, folio->mapping == NODE_MAPPING(sbi) &&
>>                 folio->index != nid_of_node(&folio->page));
> 
> Well, syzkaller is very "creative" in generating weirdly damaged filesystems :-).

Yeah.

> 
> An overall idea of the patch is that an attempt to complete I/O on a filesystem
> which is known to be "broken enough to run fsck" may make the things even worse
> (by making the filesystem even less consistent). So it may be safer just to
> refuse all in-flight write attempts at least.

We will handle IO once there is critical error (tagged w/ CP_ERROR_FLAG), you
can check the detailed behaviors as below:

errors=%s                Specify f2fs behavior on critical errors. This supports modes:
                         "panic", "continue" and "remount-ro", respectively, trigger
                         panic immediately, continue without doing anything, and remount
                         the partition in read-only mode. By default it uses "continue"
                         mode.
                         ====================== =============== =============== ========
                         mode                   continue        remount-ro      panic
                         ====================== =============== =============== ========
                         access ops             normal          normal          N/A
                         syscall errors         -EIO            -EROFS          N/A
                         mount option           rw              ro              N/A
                         pending dir write      keep            keep            N/A
                         pending non-dir write  drop            keep            N/A
                         pending node write     drop            keep            N/A
                         pending meta write     keep            keep            N/A
                         ====================== =============== =============== ========

But for SBI_NEED_FSCK case, if the inconsistency is detected in specific inode, can we just
stop further updates for this inode? Rather than all inodes?

Thanks,

> 
> Dmitry
> 



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] fs: f2fs: end all corrupted fs writes with -EFSCORRUPTED

[Chao Yu via Linux-f2fs-devel]

Jaegeuk,

I guess we need to drop this patch from dev-test, since xfstests may suffer hang
issue w/ it.

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev-test&id=fb7e51d19b3dc00bb8e3887399eb3e8f8aa1d078

Thanks,

On 4/25/25 10:29, Chao Yu wrote:
> On 4/24/25 00:15, Dmitry Antipov wrote:
>> In 'f2fs_write_end_io()', end all of the in-progress writes with
>> -EFSCORRUPTED if the corresponding error was detected earlier.
>>
>> Reported-by: syzbot+803dd716c4310d16ff3a@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=803dd716c4310d16ff3a
>> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>> ---
>>  fs/f2fs/data.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 54f89f0ee69b..755806868f0e 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -320,9 +320,11 @@ static void f2fs_write_end_io(struct bio *bio)
>>  {
>>  	struct f2fs_sb_info *sbi;
>>  	struct folio_iter fi;
>> +	bool check;
>>  
>>  	iostat_update_and_unbind_ctx(bio);
>>  	sbi = bio->bi_private;
>> +	check = is_sbi_flag_set(sbi, SBI_NEED_FSCK);
>>  
>>  	if (time_to_inject(sbi, FAULT_WRITE_IO))
>>  		bio->bi_status = BLK_STS_IOERR;
>> @@ -331,6 +333,11 @@ static void f2fs_write_end_io(struct bio *bio)
>>  		struct folio *folio = fi.folio;
>>  		enum count_type type;
>>  
>> +		if (check) {
>> +			mapping_set_error(folio->mapping, -EFSCORRUPTED);
>> +			continue;
> 
> Backing to the implementation, it seems it won't release resources
> below, e.g. missed to call:
> 
> - fscrypt_free_bounce_page()
> - f2fs_compress_write_end_io()
> ...
> - folio_end_writeback()
> 
> I guess it may hang kernel w/ this patch? Or any thing I missed?
> 
> Thanks,
> 
>> +		}
>> +
>>  		if (fscrypt_is_bounce_folio(folio)) {
>>  			struct folio *io_folio = folio;
>>  
> 



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel