[PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[Yunlei He]

-write_checkpoint
   -do_checkpoint
      -next_free_nid    <--- something wrong with next free nid

-f2fs_fill_super
   -build_node_manager
      -build_free_nids
          -get_current_nat_page
             -__get_meta_page   <--- attempt to access beyond end of device

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/node.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index b99e318..98351a4 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1955,6 +1955,9 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
 	int i = 0;
 	nid_t nid = nm_i->next_scan_nid;
 
+	if (unlikely(nid >= nm_i->max_nid))
+		nid = 0;
+
 	/* Enough entries */
 	if (nm_i->nid_cnt[FREE_NID_LIST] >= NAT_ENTRY_PER_BLOCK)
 		return;
-- 
2.10.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[Chao Yu]

Hi all,

On 2017/4/26 15:56, Yunlei He wrote:
> -write_checkpoint
>    -do_checkpoint
>       -next_free_nid    <--- something wrong with next free nid

Since we have figured out how next_scan_nid can be changed to a wrong value, so
how about add a bug_on here to detect potential issue for further fixing?

Thanks,

> 
> -f2fs_fill_super
>    -build_node_manager
>       -build_free_nids
>           -get_current_nat_page
>              -__get_meta_page   <--- attempt to access beyond end of device
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/node.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index b99e318..98351a4 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1955,6 +1955,9 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
>  	int i = 0;
>  	nid_t nid = nm_i->next_scan_nid;
>  
> +	if (unlikely(nid >= nm_i->max_nid))
> +		nid = 0;
> +
>  	/* Enough entries */
>  	if (nm_i->nid_cnt[FREE_NID_LIST] >= NAT_ENTRY_PER_BLOCK)
>  		return;
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[Jaegeuk Kim]

On 05/03, Chao Yu wrote:
> Hi all,
> 
> On 2017/4/26 15:56, Yunlei He wrote:
> > -write_checkpoint
> >    -do_checkpoint
> >       -next_free_nid    <--- something wrong with next free nid
> 
> Since we have figured out how next_scan_nid can be changed to a wrong value, so
> how about add a bug_on here to detect potential issue for further fixing?

I'll drop this patch for next merge window.
So, what was the root cause of this?

Thanks,

> 
> Thanks,
> 
> > 
> > -f2fs_fill_super
> >    -build_node_manager
> >       -build_free_nids
> >           -get_current_nat_page
> >              -__get_meta_page   <--- attempt to access beyond end of device
> > 
> > Signed-off-by: Yunlei He <heyunlei@huawei.com>
> > ---
> >  fs/f2fs/node.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index b99e318..98351a4 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1955,6 +1955,9 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
> >  	int i = 0;
> >  	nid_t nid = nm_i->next_scan_nid;
> >  
> > +	if (unlikely(nid >= nm_i->max_nid))
> > +		nid = 0;
> > +
> >  	/* Enough entries */
> >  	if (nm_i->nid_cnt[FREE_NID_LIST] >= NAT_ENTRY_PER_BLOCK)
> >  		return;
> > 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[Chao Yu]

On 2017/5/4 1:04, Jaegeuk Kim wrote:
> On 05/03, Chao Yu wrote:
>> Hi all,
>>
>> On 2017/4/26 15:56, Yunlei He wrote:
>>> -write_checkpoint
>>>    -do_checkpoint
>>>       -next_free_nid    <--- something wrong with next free nid
>>
>> Since we have figured out how next_scan_nid can be changed to a wrong value, so
>> how about add a bug_on here to detect potential issue for further fixing?
> 
> I'll drop this patch for next merge window.
> So, what was the root cause of this?

I don't know yet, :( maybe it was caused by bit-transition of cpu or memory.

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> -f2fs_fill_super
>>>    -build_node_manager
>>>       -build_free_nids
>>>           -get_current_nat_page
>>>              -__get_meta_page   <--- attempt to access beyond end of device
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/node.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index b99e318..98351a4 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -1955,6 +1955,9 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
>>>  	int i = 0;
>>>  	nid_t nid = nm_i->next_scan_nid;
>>>  
>>> +	if (unlikely(nid >= nm_i->max_nid))
>>> +		nid = 0;
>>> +
>>>  	/* Enough entries */
>>>  	if (nm_i->nid_cnt[FREE_NID_LIST] >= NAT_ENTRY_PER_BLOCK)
>>>  		return;
>>>
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[Jaegeuk Kim]

On 05/04, Chao Yu wrote:
> On 2017/5/4 1:04, Jaegeuk Kim wrote:
> > On 05/03, Chao Yu wrote:
> >> Hi all,
> >>
> >> On 2017/4/26 15:56, Yunlei He wrote:
> >>> -write_checkpoint
> >>>    -do_checkpoint
> >>>       -next_free_nid    <--- something wrong with next free nid
> >>
> >> Since we have figured out how next_scan_nid can be changed to a wrong value, so
> >> how about add a bug_on here to detect potential issue for further fixing?
> > 
> > I'll drop this patch for next merge window.
> > So, what was the root cause of this?
> 
> I don't know yet, :( maybe it was caused by bit-transition of cpu or memory.

Hmm, then this patch would be just fine, as I think. Even if nm_i->next_scan_nid
was corrupted, we can set it back as normal.

Thanks,

> 
> Thanks,
> 
> > 
> > Thanks,
> > 
> >>
> >> Thanks,
> >>
> >>>
> >>> -f2fs_fill_super
> >>>    -build_node_manager
> >>>       -build_free_nids
> >>>           -get_current_nat_page
> >>>              -__get_meta_page   <--- attempt to access beyond end of device
> >>>
> >>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >>> ---
> >>>  fs/f2fs/node.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >>> index b99e318..98351a4 100644
> >>> --- a/fs/f2fs/node.c
> >>> +++ b/fs/f2fs/node.c
> >>> @@ -1955,6 +1955,9 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
> >>>  	int i = 0;
> >>>  	nid_t nid = nm_i->next_scan_nid;
> >>>  
> >>> +	if (unlikely(nid >= nm_i->max_nid))
> >>> +		nid = 0;
> >>> +
> >>>  	/* Enough entries */
> >>>  	if (nm_i->nid_cnt[FREE_NID_LIST] >= NAT_ENTRY_PER_BLOCK)
> >>>  		return;
> >>>
> > 
> > .
> > 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: fix a mount fail for wrong next_scan_nid

[Chao Yu]

On 2017/5/4 10:24, Jaegeuk Kim wrote:
> On 05/04, Chao Yu wrote:
>> On 2017/5/4 1:04, Jaegeuk Kim wrote:
>>> On 05/03, Chao Yu wrote:
>>>> Hi all,
>>>>
>>>> On 2017/4/26 15:56, Yunlei He wrote:
>>>>> -write_checkpoint
>>>>>    -do_checkpoint
>>>>>       -next_free_nid    <--- something wrong with next free nid
>>>>
>>>> Since we have figured out how next_scan_nid can be changed to a wrong value, so
>>>> how about add a bug_on here to detect potential issue for further fixing?
>>>
>>> I'll drop this patch for next merge window.
>>> So, what was the root cause of this?
>>
>> I don't know yet, :( maybe it was caused by bit-transition of cpu or memory.
> 
> Hmm, then this patch would be just fine, as I think. Even if nm_i->next_scan_nid
> was corrupted, we can set it back as normal.

IMO, for product, it's better to set it back, as we expect filesystem module has
fault tolerance itself, for upstream, it would be good to keep bug_on to  detect
further bug. Anyway, not a big deal. ;)

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> -f2fs_fill_super
>>>>>    -build_node_manager
>>>>>       -build_free_nids
>>>>>           -get_current_nat_page
>>>>>              -__get_meta_page   <--- attempt to access beyond end of device
>>>>>
>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>> ---
>>>>>  fs/f2fs/node.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>>>> index b99e318..98351a4 100644
>>>>> --- a/fs/f2fs/node.c
>>>>> +++ b/fs/f2fs/node.c
>>>>> @@ -1955,6 +1955,9 @@ static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
>>>>>  	int i = 0;
>>>>>  	nid_t nid = nm_i->next_scan_nid;
>>>>>  
>>>>> +	if (unlikely(nid >= nm_i->max_nid))
>>>>> +		nid = 0;
>>>>> +
>>>>>  	/* Enough entries */
>>>>>  	if (nm_i->nid_cnt[FREE_NID_LIST] >= NAT_ENTRY_PER_BLOCK)
>>>>>  		return;
>>>>>
>>>
>>> .
>>>
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot