[f2fs-dev] [PATCH v2 1/2] f2fs: fix zero-sized extent for precache extents

[f2fs-dev] [PATCH v2 1/2] f2fs: fix zero-sized extent for precache extents

[wangzijie]

Script to reproduce:
f2fs_io write 1 0 1881 rand dsync testfile
f2fs_io fallocate 0 7708672 4096 testfile
f2fs_io write 1 1881 1 rand buffered testfile
fsync testfile
umount
mount
f2fs_io precache_extents testfile

When the data layout is something like this:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

During precache_extents, we map the last block(valid blkaddr) in dnode1:
map->m_flags |= F2FS_MAP_MAPPED;
map->m_pblk = blkaddr(valid blkaddr);
map->m_len = 1;
then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
map->m_flags & F2FS_MAP_MAPPED == true, and we make zero-sized extent.

Rebased on patch[1], this patch can cover these cases to avoid zero-sized extent:
A,B,C is valid blkaddr
case1:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

case2:
dnode1:                     dnode2:
[0]      A                  [0]    C (C!=B+1)
[1]      A+1                [1]    C+1
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

case3:
dnode1:                     dnode2:
[0]      A                  [0]    C (C!=B+2)
[1]      A+1                [1]    C+1
...                         ....
[1015]   A+1015
[1016]   B (B!=A+1016)
[1017]   B+1                [1017] 0x0

[1] https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org/

Fixes: c4020b2da4c9 ("f2fs: support F2FS_IOC_PRECACHE_EXTENTS")
Signed-off-by: wangzijie <wangzijie1@honor.com>
---
Rebased on:
https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org/
v1:
https://lore.kernel.org/linux-f2fs-devel/20250912103915.3597413-1-wangzijie1@honor.com
---
 fs/f2fs/data.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 838eae39d..7a5170b32 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1778,9 +1778,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 		if (map->m_flags & F2FS_MAP_MAPPED) {
 			unsigned int ofs = start_pgofs - map->m_lblk;
 
-			f2fs_update_read_extent_cache_range(&dn,
-				start_pgofs, map->m_pblk + ofs,
-				map->m_len - ofs);
+			if (map->m_len - ofs > 0)
+				f2fs_update_read_extent_cache_range(&dn,
+					start_pgofs, map->m_pblk + ofs,
+					map->m_len - ofs);
 		}
 		if (map->m_next_extent)
 			*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
-- 
2.25.1



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

[f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

When we get wrong extent info data, and look up extent_node in rb tree,
it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
return NULL.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
 fs/f2fs/extent_cache.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 199c1e7a8..6ed6f3d1d 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 			leftmost = false;
 		} else {
 			f2fs_bug_on(sbi, 1);
+			return NULL;
 		}
 	}
 
-- 
2.25.1



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[Chao Yu via Linux-f2fs-devel]

On 9/15/25 11:52, wangzijie wrote:
> When we get wrong extent info data, and look up extent_node in rb tree,
> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
> return NULL.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[Jaegeuk Kim via Linux-f2fs-devel]

On 09/15, wangzijie wrote:
> When we get wrong extent info data, and look up extent_node in rb tree,
> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
> return NULL.

This is the exact buggy case which we should fix the original one. Have
you seen this error? In that case, can we consider writing some kernel
message and handle the error properly?


> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/extent_cache.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 199c1e7a8..6ed6f3d1d 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>  			leftmost = false;
>  		} else {
>  			f2fs_bug_on(sbi, 1);
> +			return NULL;
>  		}
>  	}
>  
> -- 
> 2.25.1


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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

>On 09/15, wangzijie wrote:
>> When we get wrong extent info data, and look up extent_node in rb tree,
>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>> return NULL.
>
>This is the exact buggy case which we should fix the original one. Have
>you seen this error? In that case, can we consider writing some kernel
>message and handle the error properly?

Hi Jaegeuk,
The original one is the bug I mentioned in the first patch of this patch set
("f2fs: fix zero-sized extent for precache extents"). 

When we use a wrong extent_info(zero-sized) to do update, and there exists a
extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
which cause the infinite loop in __insert_extent_tree().

So we can add f2fs_bug_on() when there occurs zero-sized extent
in f2fs_update_read_extent_cache_range(), and give up this zero-sized
extent update to handle other unknown buggy cases. Do you think this will be better?

And do we need to solve this infinite loop?


>> 
>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>> ---
>>  fs/f2fs/extent_cache.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index 199c1e7a8..6ed6f3d1d 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>  			leftmost = false;
>>  		} else {
>>  			f2fs_bug_on(sbi, 1);
>> +			return NULL;
>>  		}
>>  	}
>>  
>> -- 
>> 2.25.1


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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[Chao Yu via Linux-f2fs-devel]

On 9/16/25 13:22, wangzijie wrote:
>> On 09/15, wangzijie wrote:
>>> When we get wrong extent info data, and look up extent_node in rb tree,
>>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>>> return NULL.
>>
>> This is the exact buggy case which we should fix the original one. Have
>> you seen this error? In that case, can we consider writing some kernel
>> message and handle the error properly?
> 
> Hi Jaegeuk,
> The original one is the bug I mentioned in the first patch of this patch set
> ("f2fs: fix zero-sized extent for precache extents"). 

Zijie,

Did you suffer this problem in product? right?

> 
> When we use a wrong extent_info(zero-sized) to do update, and there exists a
> extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
> nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
> which cause the infinite loop in __insert_extent_tree().
> 
> So we can add f2fs_bug_on() when there occurs zero-sized extent
> in f2fs_update_read_extent_cache_range(), and give up this zero-sized
> extent update to handle other unknown buggy cases. Do you think this will be better?
> 
> And do we need to solve this infinite loop?

IMO, it's worth to end such loop if there is any corrupted extent in rbtree to
avoid kernel hang, no matter it is caused by software bug or hardware flaw
potentially.

Thanks,

> 
> 
>>>
>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>> ---
>>>  fs/f2fs/extent_cache.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>> index 199c1e7a8..6ed6f3d1d 100644
>>> --- a/fs/f2fs/extent_cache.c
>>> +++ b/fs/f2fs/extent_cache.c
>>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>>  			leftmost = false;
>>>  		} else {
>>>  			f2fs_bug_on(sbi, 1);
>>> +			return NULL;
>>>  		}
>>>  	}
>>>  
>>> -- 
>>> 2.25.1



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

>On 9/16/25 13:22, wangzijie wrote:
>>> On 09/15, wangzijie wrote:
>>>> When we get wrong extent info data, and look up extent_node in rb tree,
>>>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>>>> return NULL.
>>>
>>> This is the exact buggy case which we should fix the original one. Have
>>> you seen this error? In that case, can we consider writing some kernel
>>> message and handle the error properly?
>> 
>> Hi Jaegeuk,
>> The original one is the bug I mentioned in the first patch of this patch set
>> ("f2fs: fix zero-sized extent for precache extents"). 
>
>Zijie,
>
>Did you suffer this problem in product? right?

Hi Chao,
Yes, and I can confirm that infinite loop cases I suffered are caused by the bug I
mentioned in the first patch of this patch set. But I'm not sure if there are
other cases that can cause this infinite loop.

>> 
>> When we use a wrong extent_info(zero-sized) to do update, and there exists a
>> extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
>> nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
>> which cause the infinite loop in __insert_extent_tree().
>> 
>> So we can add f2fs_bug_on() when there occurs zero-sized extent
>> in f2fs_update_read_extent_cache_range(), and give up this zero-sized
>> extent update to handle other unknown buggy cases. Do you think this will be better?
>> 
>> And do we need to solve this infinite loop?
>
>IMO, it's worth to end such loop if there is any corrupted extent in rbtree to
>avoid kernel hang, no matter it is caused by software bug or hardware flaw
>potentially.
>
>Thanks,

And do you think we need this?
"add f2fs_bug_on() when there occurs zero-sized extent in f2fs_update_read_extent_cache_range(),
and give up this zero-sized extent update to handle other unknown buggy cases".



>> 
>> 
>>>>
>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>> ---
>>>>  fs/f2fs/extent_cache.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>> index 199c1e7a8..6ed6f3d1d 100644
>>>> --- a/fs/f2fs/extent_cache.c
>>>> +++ b/fs/f2fs/extent_cache.c
>>>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>>>  			leftmost = false;
>>>>  		} else {
>>>>  			f2fs_bug_on(sbi, 1);
>>>> +			return NULL;
>>>>  		}
>>>>  	}
>>>>  
>>>> -- 
>>>> 2.25.1



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[Chao Yu via Linux-f2fs-devel]

On 9/16/25 15:09, wangzijie wrote:
>> On 9/16/25 13:22, wangzijie wrote:
>>>> On 09/15, wangzijie wrote:
>>>>> When we get wrong extent info data, and look up extent_node in rb tree,
>>>>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>>>>> return NULL.
>>>>
>>>> This is the exact buggy case which we should fix the original one. Have
>>>> you seen this error? In that case, can we consider writing some kernel
>>>> message and handle the error properly?
>>>
>>> Hi Jaegeuk,
>>> The original one is the bug I mentioned in the first patch of this patch set
>>> ("f2fs: fix zero-sized extent for precache extents"). 
>>
>> Zijie,
>>
>> Did you suffer this problem in product? right?
> 
> Hi Chao,
> Yes, and I can confirm that infinite loop cases I suffered are caused by the bug I
> mentioned in the first patch of this patch set. But I'm not sure if there are
> other cases that can cause this infinite loop.
> 
>>>
>>> When we use a wrong extent_info(zero-sized) to do update, and there exists a
>>> extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
>>> nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
>>> which cause the infinite loop in __insert_extent_tree().
>>>
>>> So we can add f2fs_bug_on() when there occurs zero-sized extent
>>> in f2fs_update_read_extent_cache_range(), and give up this zero-sized
>>> extent update to handle other unknown buggy cases. Do you think this will be better?
>>>
>>> And do we need to solve this infinite loop?
>>
>> IMO, it's worth to end such loop if there is any corrupted extent in rbtree to
>> avoid kernel hang, no matter it is caused by software bug or hardware flaw
>> potentially.
>>
>> Thanks,
> 
> And do you think we need this?
> "add f2fs_bug_on() when there occurs zero-sized extent in f2fs_update_read_extent_cache_range(),
> and give up this zero-sized extent update to handle other unknown buggy cases".

Oh, I was testing below patch..., does this what you want to do?

I think we can keep all your patches, and appending below patch to detect any
potential cases who will update a zero-sized extent.

From 439d61ef3715fafa5c9f2d1b7f8026cdd2564ca7 Mon Sep 17 00:00:00 2001
From: Chao Yu <chao@kernel.org>
Date: Tue, 16 Sep 2025 11:52:30 +0800
Subject: [PATCH] f2fs: add sanity check on ei.len in
 __update_extent_tree_range()

Add a sanity check in __update_extent_tree_range() to detect any
zero-sized extent update.

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/extent_cache.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 199c1e7a83ef..9544323767be 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -664,6 +664,15 @@ static void __update_extent_tree_range(struct inode *inode,
 	if (!et)
 		return;

+	if (unlikely(len == 0)) {
+		f2fs_bug_on(sbi, 1);
+		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
+			"extent [%u, %u, %u], age [%llu, %llu]",
+			__func__, type, tei->fofs, tei->blk, tei->len,
+			tei->age, tei->last_blocks);
+		return;
+	}
+
 	if (type == EX_READ)
 		trace_f2fs_update_read_extent_tree_range(inode, fofs, len,
 						tei->blk, 0);
-- 
2.49.0


> 
> 
> 
>>>
>>>
>>>>>
>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>> ---
>>>>>  fs/f2fs/extent_cache.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>> index 199c1e7a8..6ed6f3d1d 100644
>>>>> --- a/fs/f2fs/extent_cache.c
>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>>>>  			leftmost = false;
>>>>>  		} else {
>>>>>  			f2fs_bug_on(sbi, 1);
>>>>> +			return NULL;
>>>>>  		}
>>>>>  	}
>>>>>  
>>>>> -- 
>>>>> 2.25.1
> 



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

>On 9/16/25 15:09, wangzijie wrote:
>>> On 9/16/25 13:22, wangzijie wrote:
>>>>> On 09/15, wangzijie wrote:
>>>>>> When we get wrong extent info data, and look up extent_node in rb tree,
>>>>>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>>>>>> return NULL.
>>>>>
>>>>> This is the exact buggy case which we should fix the original one. Have
>>>>> you seen this error? In that case, can we consider writing some kernel
>>>>> message and handle the error properly?
>>>>
>>>> Hi Jaegeuk,
>>>> The original one is the bug I mentioned in the first patch of this patch set
>>>> ("f2fs: fix zero-sized extent for precache extents"). 
>>>
>>> Zijie,
>>>
>>> Did you suffer this problem in product? right?
>> 
>> Hi Chao,
>> Yes, and I can confirm that infinite loop cases I suffered are caused by the bug I
>> mentioned in the first patch of this patch set. But I'm not sure if there are
>> other cases that can cause this infinite loop.
>> 
>>>>
>>>> When we use a wrong extent_info(zero-sized) to do update, and there exists a
>>>> extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
>>>> nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
>>>> which cause the infinite loop in __insert_extent_tree().
>>>>
>>>> So we can add f2fs_bug_on() when there occurs zero-sized extent
>>>> in f2fs_update_read_extent_cache_range(), and give up this zero-sized
>>>> extent update to handle other unknown buggy cases. Do you think this will be better?
>>>>
>>>> And do we need to solve this infinite loop?
>>>
>>> IMO, it's worth to end such loop if there is any corrupted extent in rbtree to
>>> avoid kernel hang, no matter it is caused by software bug or hardware flaw
>>> potentially.
>>>
>>> Thanks,
>> 
>> And do you think we need this?
>> "add f2fs_bug_on() when there occurs zero-sized extent in f2fs_update_read_extent_cache_range(),
>> and give up this zero-sized extent update to handle other unknown buggy cases".
>
>Oh, I was testing below patch..., does this what you want to do?
>
>I think we can keep all your patches, and appending below patch to detect any
>potential cases who will update a zero-sized extent.
>
>>From 439d61ef3715fafa5c9f2d1b7f8026cdd2564ca7 Mon Sep 17 00:00:00 2001
>From: Chao Yu <chao@kernel.org>
>Date: Tue, 16 Sep 2025 11:52:30 +0800
>Subject: [PATCH] f2fs: add sanity check on ei.len in
> __update_extent_tree_range()
>
>Add a sanity check in __update_extent_tree_range() to detect any
>zero-sized extent update.
>
>Signed-off-by: Chao Yu <chao@kernel.org>
>---
> fs/f2fs/extent_cache.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>index 199c1e7a83ef..9544323767be 100644
>--- a/fs/f2fs/extent_cache.c
>+++ b/fs/f2fs/extent_cache.c
>@@ -664,6 +664,15 @@ static void __update_extent_tree_range(struct inode *inode,
> 	if (!et)
> 		return;
>
>+	if (unlikely(len == 0)) {
>+		f2fs_bug_on(sbi, 1);
>+		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
>+			"extent [%u, %u, %u], age [%llu, %llu]",
>+			__func__, type, tei->fofs, tei->blk, tei->len,
>+			tei->age, tei->last_blocks);
>+		return;
>+	}
>+
> 	if (type == EX_READ)
> 		trace_f2fs_update_read_extent_tree_range(inode, fofs, len,
> 						tei->blk, 0);
>-- 
>2.49.0

Yes, that's exactly what I want to do.
Maybe we should relocate f2fs_bug_on()?

	if (unlikely(len == 0)) {
		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
			"extent [%u, %u, %u], age [%llu, %llu]",
			__func__, type, tei->fofs, tei->blk, tei->len,
			tei->age, tei->last_blocks);
		f2fs_bug_on(sbi, 1);
		return;
	}

>> 
>> 
>> 
>>>>
>>>>
>>>>>>
>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>> ---
>>>>>>  fs/f2fs/extent_cache.c | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>> index 199c1e7a8..6ed6f3d1d 100644
>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>>>>>  			leftmost = false;
>>>>>>  		} else {
>>>>>>  			f2fs_bug_on(sbi, 1);
>>>>>> +			return NULL;
>>>>>>  		}
>>>>>>  	}
>>>>>>  
>>>>>> -- 
>>>>>> 2.25.1
>> 



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[Chao Yu via Linux-f2fs-devel]

On 9/16/25 16:26, wangzijie wrote:
>> On 9/16/25 15:09, wangzijie wrote:
>>>> On 9/16/25 13:22, wangzijie wrote:
>>>>>> On 09/15, wangzijie wrote:
>>>>>>> When we get wrong extent info data, and look up extent_node in rb tree,
>>>>>>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>>>>>>> return NULL.
>>>>>>
>>>>>> This is the exact buggy case which we should fix the original one. Have
>>>>>> you seen this error? In that case, can we consider writing some kernel
>>>>>> message and handle the error properly?
>>>>>
>>>>> Hi Jaegeuk,
>>>>> The original one is the bug I mentioned in the first patch of this patch set
>>>>> ("f2fs: fix zero-sized extent for precache extents"). 
>>>>
>>>> Zijie,
>>>>
>>>> Did you suffer this problem in product? right?
>>>
>>> Hi Chao,
>>> Yes, and I can confirm that infinite loop cases I suffered are caused by the bug I
>>> mentioned in the first patch of this patch set. But I'm not sure if there are
>>> other cases that can cause this infinite loop.
>>>
>>>>>
>>>>> When we use a wrong extent_info(zero-sized) to do update, and there exists a
>>>>> extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
>>>>> nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
>>>>> which cause the infinite loop in __insert_extent_tree().
>>>>>
>>>>> So we can add f2fs_bug_on() when there occurs zero-sized extent
>>>>> in f2fs_update_read_extent_cache_range(), and give up this zero-sized
>>>>> extent update to handle other unknown buggy cases. Do you think this will be better?
>>>>>
>>>>> And do we need to solve this infinite loop?
>>>>
>>>> IMO, it's worth to end such loop if there is any corrupted extent in rbtree to
>>>> avoid kernel hang, no matter it is caused by software bug or hardware flaw
>>>> potentially.
>>>>
>>>> Thanks,
>>>
>>> And do you think we need this?
>>> "add f2fs_bug_on() when there occurs zero-sized extent in f2fs_update_read_extent_cache_range(),
>>> and give up this zero-sized extent update to handle other unknown buggy cases".
>>
>> Oh, I was testing below patch..., does this what you want to do?
>>
>> I think we can keep all your patches, and appending below patch to detect any
>> potential cases who will update a zero-sized extent.
>>
>> >From 439d61ef3715fafa5c9f2d1b7f8026cdd2564ca7 Mon Sep 17 00:00:00 2001
>> From: Chao Yu <chao@kernel.org>
>> Date: Tue, 16 Sep 2025 11:52:30 +0800
>> Subject: [PATCH] f2fs: add sanity check on ei.len in
>> __update_extent_tree_range()
>>
>> Add a sanity check in __update_extent_tree_range() to detect any
>> zero-sized extent update.
>>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> fs/f2fs/extent_cache.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index 199c1e7a83ef..9544323767be 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -664,6 +664,15 @@ static void __update_extent_tree_range(struct inode *inode,
>> 	if (!et)
>> 		return;
>>
>> +	if (unlikely(len == 0)) {
>> +		f2fs_bug_on(sbi, 1);
>> +		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
>> +			"extent [%u, %u, %u], age [%llu, %llu]",
>> +			__func__, type, tei->fofs, tei->blk, tei->len,
>> +			tei->age, tei->last_blocks);
>> +		return;
>> +	}
>> +
>> 	if (type == EX_READ)
>> 		trace_f2fs_update_read_extent_tree_range(inode, fofs, len,
>> 						tei->blk, 0);
>> -- 
>> 2.49.0
> 
> Yes, that's exactly what I want to do.
> Maybe we should relocate f2fs_bug_on()?
> 
> 	if (unlikely(len == 0)) {
> 		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
> 			"extent [%u, %u, %u], age [%llu, %llu]",
> 			__func__, type, tei->fofs, tei->blk, tei->len,
> 			tei->age, tei->last_blocks);
> 		f2fs_bug_on(sbi, 1);
> 		return;
> 	}

Yeah, looks better.

I don't see any problem in my test, will send a formal patch, let me add
Signed-off-by of you if you don't mind. :)

Thanks,

> 
>>>
>>>
>>>
>>>>>
>>>>>
>>>>>>>
>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>> ---
>>>>>>>  fs/f2fs/extent_cache.c | 1 +
>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>>> index 199c1e7a8..6ed6f3d1d 100644
>>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>>>>>>  			leftmost = false;
>>>>>>>  		} else {
>>>>>>>  			f2fs_bug_on(sbi, 1);
>>>>>>> +			return NULL;
>>>>>>>  		}
>>>>>>>  	}
>>>>>>>  
>>>>>>> -- 
>>>>>>> 2.25.1
>>>
> 



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

>On 9/16/25 16:26, wangzijie wrote:
>>> On 9/16/25 15:09, wangzijie wrote:
>>>>> On 9/16/25 13:22, wangzijie wrote:
>>>>>>> On 09/15, wangzijie wrote:
>>>>>>>> When we get wrong extent info data, and look up extent_node in rb tree,
>>>>>>>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>>>>>>>> return NULL.
>>>>>>>
>>>>>>> This is the exact buggy case which we should fix the original one. Have
>>>>>>> you seen this error? In that case, can we consider writing some kernel
>>>>>>> message and handle the error properly?
>>>>>>
>>>>>> Hi Jaegeuk,
>>>>>> The original one is the bug I mentioned in the first patch of this patch set
>>>>>> ("f2fs: fix zero-sized extent for precache extents"). 
>>>>>
>>>>> Zijie,
>>>>>
>>>>> Did you suffer this problem in product? right?
>>>>
>>>> Hi Chao,
>>>> Yes, and I can confirm that infinite loop cases I suffered are caused by the bug I
>>>> mentioned in the first patch of this patch set. But I'm not sure if there are
>>>> other cases that can cause this infinite loop.
>>>>
>>>>>>
>>>>>> When we use a wrong extent_info(zero-sized) to do update, and there exists a
>>>>>> extent_node which has same fofs as the wrong one, we will skip "invalidate all extent
>>>>>> nodes in range [fofs, fofs + len - 1]"(en->ei.fofs = end = tei->fofs + tei->len = tei->fofs),
>>>>>> which cause the infinite loop in __insert_extent_tree().
>>>>>>
>>>>>> So we can add f2fs_bug_on() when there occurs zero-sized extent
>>>>>> in f2fs_update_read_extent_cache_range(), and give up this zero-sized
>>>>>> extent update to handle other unknown buggy cases. Do you think this will be better?
>>>>>>
>>>>>> And do we need to solve this infinite loop?
>>>>>
>>>>> IMO, it's worth to end such loop if there is any corrupted extent in rbtree to
>>>>> avoid kernel hang, no matter it is caused by software bug or hardware flaw
>>>>> potentially.
>>>>>
>>>>> Thanks,
>>>>
>>>> And do you think we need this?
>>>> "add f2fs_bug_on() when there occurs zero-sized extent in f2fs_update_read_extent_cache_range(),
>>>> and give up this zero-sized extent update to handle other unknown buggy cases".
>>>
>>> Oh, I was testing below patch..., does this what you want to do?
>>>
>>> I think we can keep all your patches, and appending below patch to detect any
>>> potential cases who will update a zero-sized extent.
>>>
>>> >From 439d61ef3715fafa5c9f2d1b7f8026cdd2564ca7 Mon Sep 17 00:00:00 2001
>>> From: Chao Yu <chao@kernel.org>
>>> Date: Tue, 16 Sep 2025 11:52:30 +0800
>>> Subject: [PATCH] f2fs: add sanity check on ei.len in
>>> __update_extent_tree_range()
>>>
>>> Add a sanity check in __update_extent_tree_range() to detect any
>>> zero-sized extent update.
>>>
>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>> ---
>>> fs/f2fs/extent_cache.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>> index 199c1e7a83ef..9544323767be 100644
>>> --- a/fs/f2fs/extent_cache.c
>>> +++ b/fs/f2fs/extent_cache.c
>>> @@ -664,6 +664,15 @@ static void __update_extent_tree_range(struct inode *inode,
>>> 	if (!et)
>>> 		return;
>>>
>>> +	if (unlikely(len == 0)) {
>>> +		f2fs_bug_on(sbi, 1);
>>> +		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
>>> +			"extent [%u, %u, %u], age [%llu, %llu]",
>>> +			__func__, type, tei->fofs, tei->blk, tei->len,
>>> +			tei->age, tei->last_blocks);
>>> +		return;
>>> +	}
>>> +
>>> 	if (type == EX_READ)
>>> 		trace_f2fs_update_read_extent_tree_range(inode, fofs, len,
>>> 						tei->blk, 0);
>>> -- 
>>> 2.49.0
>> 
>> Yes, that's exactly what I want to do.
>> Maybe we should relocate f2fs_bug_on()?
>> 
>> 	if (unlikely(len == 0)) {
>> 		f2fs_err_ratelimited(sbi, "%s: extent len is zero, type: %d, "
>> 			"extent [%u, %u, %u], age [%llu, %llu]",
>> 			__func__, type, tei->fofs, tei->blk, tei->len,
>> 			tei->age, tei->last_blocks);
>> 		f2fs_bug_on(sbi, 1);
>> 		return;
>> 	}
>
>Yeah, looks better.
>
>I don't see any problem in my test, will send a formal patch, let me add
>Signed-off-by of you if you don't mind. :)
>
>Thanks,

OK, thanks for your help.

>> 
>>>>
>>>>
>>>>
>>>>>>
>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>>> ---
>>>>>>>>  fs/f2fs/extent_cache.c | 1 +
>>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>>>> index 199c1e7a8..6ed6f3d1d 100644
>>>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>>>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>>>>>>>  			leftmost = false;
>>>>>>>>  		} else {
>>>>>>>>  			f2fs_bug_on(sbi, 1);
>>>>>>>> +			return NULL;
>>>>>>>>  		}
>>>>>>>>  	}
>>>>>>>>  
>>>>>>>> -- 
>>>>>>>> 2.25.1
>>>>
>> 


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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[Chao Yu via Linux-f2fs-devel]

On 9/15/25 11:52, wangzijie wrote:
> When we get wrong extent info data, and look up extent_node in rb tree,
> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
> return NULL.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/extent_cache.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 199c1e7a8..6ed6f3d1d 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>  			leftmost = false;
>  		} else {

Need to print some messages here as Jaegeuk suggested, IIUC.

Thanks,

>  			f2fs_bug_on(sbi, 1);
> +			return NULL;
>  		}
>  	}
>  



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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

>On 9/15/25 11:52, wangzijie wrote:
>> When we get wrong extent info data, and look up extent_node in rb tree,
>> it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
>> return NULL.
>> 
>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>> ---
>>  fs/f2fs/extent_cache.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index 199c1e7a8..6ed6f3d1d 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>>  			leftmost = false;
>>  		} else {
>
>Need to print some messages here as Jaegeuk suggested, IIUC.
>
>Thanks,

OK, I will update it in next version.

>>  			f2fs_bug_on(sbi, 1);
>> +			return NULL;
>>  		}
>>  	}
>>  


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

Re: [f2fs-dev] [PATCH v2 1/2] f2fs: fix zero-sized extent for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/15/25 11:52, wangzijie wrote:
> Script to reproduce:
> f2fs_io write 1 0 1881 rand dsync testfile
> f2fs_io fallocate 0 7708672 4096 testfile
> f2fs_io write 1 1881 1 rand buffered testfile
> fsync testfile
> umount
> mount
> f2fs_io precache_extents testfile
> 
> When the data layout is something like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> During precache_extents, we map the last block(valid blkaddr) in dnode1:
> map->m_flags |= F2FS_MAP_MAPPED;
> map->m_pblk = blkaddr(valid blkaddr);
> map->m_len = 1;
> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
> map->m_flags & F2FS_MAP_MAPPED == true, and we make zero-sized extent.
> 
> Rebased on patch[1], this patch can cover these cases to avoid zero-sized extent:
> A,B,C is valid blkaddr
> case1:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> case2:
> dnode1:                     dnode2:
> [0]      A                  [0]    C (C!=B+1)
> [1]      A+1                [1]    C+1
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> case3:
> dnode1:                     dnode2:
> [0]      A                  [0]    C (C!=B+2)
> [1]      A+1                [1]    C+1
> ...                         ....
> [1015]   A+1015
> [1016]   B (B!=A+1016)
> [1017]   B+1                [1017] 0x0
> 
> [1] https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org/
> 
> Fixes: c4020b2da4c9 ("f2fs: support F2FS_IOC_PRECACHE_EXTENTS")
> Signed-off-by: wangzijie <wangzijie1@honor.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


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