diff mbox series

[4.19,026/239] f2fs: fix to do sanity check in is_alive()

Message ID 20220124183943.957395248@linuxfoundation.org
State New
Headers show
Series None | expand

Commit Message

Greg KH Jan. 24, 2022, 6:41 p.m. UTC 
From: Chao Yu <chao@kernel.org>

commit 77900c45ee5cd5da63bd4d818a41dbdf367e81cd upstream.

In fuzzed image, SSA table may indicate that a data block belongs to
invalid node, which node ID is out-of-range (0, 1, 2 or max_nid), in
order to avoid migrating inconsistent data in such corrupted image,
let's do sanity check anyway before data block migration.

Cc: stable@vger.kernel.org
Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/f2fs/gc.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Pavel Machek Feb. 1, 2022, 7:18 p.m. UTC | #1 
Hi!

> Oops, you're right, my bad.
> 
> > 
> > > +++ b/fs/f2fs/gc.c
> > > @@ -589,6 +589,9 @@ static bool is_alive(struct f2fs_sb_info
> > >   		set_sbi_flag(sbi, SBI_NEED_FSCK);
> > >   	}
> > > +	if (f2fs_check_nid_range(sbi, dni->ino))
> > > +		return false;
> > > +
> > >   	*nofs = ofs_of_node(node_page);
> > >   	source_blkaddr = datablock_addr(NULL, node_page, ofs_in_node);
> > >   	f2fs_put_page(node_page, 1);
> > 
> > AFAICT f2fs_put_page() needs to be done in the error path, too.
> > 
> > (Problem seems to exist in mainline, too).
> > 
> > Something like this?
> 
> Could you please send a formal patch to f2fs mailing list for better review?
> 
> Anyway, thanks a lot for the report and the patch!

I'm quite busy with other reviews at the moment. If you could submit a
patch, it would be great, otherwise I'll get to it .. sometime.

Best regards,
									Pavel
Chao Yu Feb. 3, 2022, 2:51 p.m. UTC | #2 
On 2022/2/2 3:18, Pavel Machek wrote:
> Hi!
> 
>> Oops, you're right, my bad.
>>
>>>
>>>> +++ b/fs/f2fs/gc.c
>>>> @@ -589,6 +589,9 @@ static bool is_alive(struct f2fs_sb_info
>>>>    		set_sbi_flag(sbi, SBI_NEED_FSCK);
>>>>    	}
>>>> +	if (f2fs_check_nid_range(sbi, dni->ino))
>>>> +		return false;
>>>> +
>>>>    	*nofs = ofs_of_node(node_page);
>>>>    	source_blkaddr = datablock_addr(NULL, node_page, ofs_in_node);
>>>>    	f2fs_put_page(node_page, 1);
>>>
>>> AFAICT f2fs_put_page() needs to be done in the error path, too.
>>>
>>> (Problem seems to exist in mainline, too).
>>>
>>> Something like this?
>>
>> Could you please send a formal patch to f2fs mailing list for better review?
>>
>> Anyway, thanks a lot for the report and the patch!
> 
> I'm quite busy with other reviews at the moment. If you could submit a
> patch, it would be great, otherwise I'll get to it .. sometime.

I've submitted a patch, could you please take a look?

Thanks,

> 
> Best regards,
> 									Pavel
>
diff mbox series

Patch

--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -589,6 +589,9 @@  static bool is_alive(struct f2fs_sb_info
 		set_sbi_flag(sbi, SBI_NEED_FSCK);
 	}
 
+	if (f2fs_check_nid_range(sbi, dni->ino))
+		return false;
+
 	*nofs = ofs_of_node(node_page);
 	source_blkaddr = datablock_addr(NULL, node_page, ofs_in_node);
 	f2fs_put_page(node_page, 1);