On Wed, Nov 3, 2021 at 10:22 PM Dongliang Mu <[email protected]> wrote: > > f2fs_fill_super > -> f2fs_build_segment_manager > -> create_discard_cmd_control > -> f2fs_start_discard_thread > > It invokes kthread_run to create a thread and run issue_discard_thread. > > However, if f2fs_build_node_manager fails, the control flow goes to > free_nm and calls f2fs_destroy_node_manager. This function will free > sbi->nm_info. However, if issue_discard_thread accesses sbi->nm_info > after the deallocation, but before the f2fs_stop_discard_thread, it will > cause UAF(Use-after-free).
This UAF only can be triggered in a small time window. Even if syzkaller generates a reproducer, it is hard to reproduce. > > -> f2fs_destroy_segment_manager > -> destroy_discard_cmd_control > -> f2fs_stop_discard_thread > > Fix this by switching the order of f2fs_build_segment_manager > and f2fs_build_node_manager. > > Signed-off-by: Dongliang Mu <[email protected]> > --- > fs/f2fs/super.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 78ebc306ee2b..1a23b64cfb74 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -4135,18 +4135,18 @@ static int f2fs_fill_super(struct super_block *sb, > void *data, int silent) > } > > /* setup f2fs internal modules */ > - err = f2fs_build_segment_manager(sbi); > - if (err) { > - f2fs_err(sbi, "Failed to initialize F2FS segment manager > (%d)", > - err); > - goto free_sm; > - } > err = f2fs_build_node_manager(sbi); > if (err) { > f2fs_err(sbi, "Failed to initialize F2FS node manager (%d)", > err); > goto free_nm; > } > + err = f2fs_build_segment_manager(sbi); > + if (err) { > + f2fs_err(sbi, "Failed to initialize F2FS segment manager > (%d)", > + err); > + goto free_sm; > + } I am not very familiar with this filesystem. If the building of node manager is based on segment manager, we can ignore this patch and try to think up other solutions to fix this bug. > > /* For write statistics */ > sbi->sectors_written_start = f2fs_get_sectors_written(sbi); > @@ -4351,10 +4351,10 @@ static int f2fs_fill_super(struct super_block *sb, > void *data, int silent) > sbi->node_inode = NULL; > free_stats: > f2fs_destroy_stats(sbi); > -free_nm: > - f2fs_destroy_node_manager(sbi); > free_sm: > f2fs_destroy_segment_manager(sbi); > +free_nm: > + f2fs_destroy_node_manager(sbi); > f2fs_destroy_post_read_wq(sbi); > stop_ckpt_thread: > f2fs_stop_ckpt_thread(sbi); > -- > 2.25.1 > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
