linux-kernel - Re: [PATCH] fs: f2fs: fix UAF in f2fs_available_free

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD-N9QVXNUEsHVaWUjT2eg9MLTX6SXru7JZm_z7PFQO=KPPg=A@mail.gmail.com>
Date:   Wed, 3 Nov 2021 22:28:00 +0800
From:   Dongliang Mu <mudongliangabcd@...il.com>
To:     Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <chao@...nel.org>
Cc:     linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] fs: f2fs: fix UAF in f2fs_available_free_memory

On Wed, Nov 3, 2021 at 10:22 PM Dongliang Mu <mudongliangabcd@...il.com> 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 <mudongliangabcd@...il.com>
> ---
>  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
>