linux-kernel - Re: [PATCH] btrfs: ref-verify: fix memory leaks

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAL3q7H72WXSSS-crLCGi=07x8F58j9k7B9Udk8k2MVwgptO27g@mail.gmail.com>
Date:   Mon, 13 Nov 2023 13:20:04 +0000
From:   Filipe Manana <fdmanana@...nel.org>
To:     Bragatheswaran Manickavel <bragathemanick0908@...il.com>
Cc:     clm@...com, josef@...icpanda.com, dsterba@...e.com,
        linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzbot+d66de4cbf532749df35f@...kaller.appspotmail.com
Subject: Re: [PATCH] btrfs: ref-verify: fix memory leaks

On Sun, Nov 12, 2023 at 4:57 PM Bragatheswaran Manickavel
<bragathemanick0908@...il.com> wrote:
>
> In btrfs_ref_tree_mod(), when !parent 're' was allocated
> through kmalloc(). In the following code, if an error occurs,
> the execution will be redirected to 'out' or 'out_unlock' and
> the function will be exited. However, on some of the paths,
> 're' are not deallocated and may lead to memory leaks.
>
> For example : lookup_block_entry() for 'be' returns null, the
> out label will be invoked. During that flow ref and ra was
> freed but not re, which can potentially lead to memleak
>
> Reported-and-tested-by: syzbot+d66de4cbf532749df35f@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d66de4cbf532749df35f
> Signed-off-by: Bragatheswaran Manickavel <bragathemanick0908@...il.com>
> ---
>  fs/btrfs/ref-verify.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/fs/btrfs/ref-verify.c b/fs/btrfs/ref-verify.c
> index 95d28497de7c..50b59b3dc474 100644
> --- a/fs/btrfs/ref-verify.c
> +++ b/fs/btrfs/ref-verify.c
> @@ -791,6 +791,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
>                         dump_ref_action(fs_info, ra);
>                         kfree(ref);
>                         kfree(ra);
> +                       kfree(re);

Here it's fine, 're' was not yet added to the rbtree (be->roots).

>                         goto out_unlock;
>                 } else if (be->num_refs == 0) {
>                         btrfs_err(fs_info,
> @@ -800,6 +801,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
>                         dump_ref_action(fs_info, ra);
>                         kfree(ref);
>                         kfree(ra);
> +                       kfree(re);

Same here.

>                         goto out_unlock;
>                 }
>
> @@ -822,6 +824,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
>                                 dump_ref_action(fs_info, ra);
>                                 kfree(ref);
>                                 kfree(ra);
> +                               kfree(re);

Here it's not ok. 're' was added to the rbtree, so you can't free it,
as later when accessing the tree, it will trigger a use-after-free
bug.

>                                 goto out_unlock;
>                         }
>                         exist->num_refs--;
> @@ -838,6 +841,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
>                         dump_ref_action(fs_info, ra);
>                         kfree(ref);
>                         kfree(ra);
> +                       kfree(re);

Same here, it will lead to a use-after-free.

>                         goto out_unlock;
>                 }
>                 kfree(ref);
> @@ -849,6 +853,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
>                         dump_ref_action(fs_info, ra);
>                         kfree(ref);
>                         kfree(ra);
> +                       kfree(re);

Same here, it will lead to a use-after-free.

Thanks.
>                         goto out_unlock;
>                 }
>         }
> --
> 2.34.1
>
>