linux-kernel - Re: [PATCH v4 3/3] mm: fix use-after-free when anon vma name is used after vma is freed

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]

Date:   Mon, 21 Feb 2022 21:41:30 -0800
From:   Suren Baghdasaryan <surenb@...gle.com>
To:     akpm@...ux-foundation.org
Cc:     ccross@...gle.com, sumit.semwal@...aro.org, mhocko@...e.com,
        dave.hansen@...el.com, keescook@...omium.org, willy@...radead.org,
        kirill.shutemov@...ux.intel.com, vbabka@...e.cz,
        hannes@...xchg.org, ebiederm@...ssion.com, brauner@...nel.org,
        legion@...nel.org, ran.xiaokai@....com.cn, sashal@...nel.org,
        chris.hyser@...cle.com, dave@...olabs.net, pcc@...gle.com,
        caoxiaofeng@...ong.com, david@...hat.com, gorcunov@...il.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        kernel-team@...roid.com,
        syzbot+aa7b3d4b35f9dc46a366@...kaller.appspotmail.com
Subject: Re: [PATCH v4 3/3] mm: fix use-after-free when anon vma name is used
 after vma is freed

On Mon, Feb 21, 2022 at 9:40 PM Suren Baghdasaryan <surenb@...gle.com> wrote:
>
> When adjacent vmas are being merged it can result in the vma that was
> originally passed to madvise_update_vma being destroyed.  In the current
> implementation, the name parameter passed to madvise_update_vma points
> directly to vma->anon_name->name and it is used after the call to
> vma_merge.  In the cases when vma_merge merges the original vma and
> destroys it, this will result in use-after-free bug as shown below:
>
> madvise_vma_behavior << passes vma->anon_name->name as name param
>   madvise_update_vma(name)
>     vma_merge
>       __vma_adjust
>         vm_area_free <-- frees the vma
>     replace_vma_anon_name(name) <-- UAF
>
> Fix this by raising the name refcount and stabilizing it.
>
> Fixes: 9a10064f5625 ("mm: add a field to store names for private anonymous memory")
> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> Reported-by: syzbot+aa7b3d4b35f9dc46a366@...kaller.appspotmail.com
> ---
> changes in v3:
> - Reapplied the fix after code refactoring, per Michal Hocko

Hi Andrew,
Since I needed to make some refactoring before adding this fix, in
order to apply this new version to mmotm you would need to revert the
previous version of this patch from your tree:
0cc16837d264 ("mm: fix use-after-free when anon vma name is used after
vma is freed")
and then apply the whole patchset (3 patches) after it is reviewed.
Sorry for the inconvenience but I think this way the refactoring and
the fix would be in the right order and with no overlap.
The patchset applies cleanly to Linus' ToT and to mmotm after
0cc16837d264 is reverted.
Thanks,
Suren.

>
>  mm/madvise.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index a395884aeecb..00e8105430e9 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -140,6 +140,8 @@ static int replace_vma_anon_name(struct vm_area_struct *vma,
>  /*
>   * Update the vm_flags on region of a vma, splitting it or merging it as
>   * necessary.  Must be called with mmap_sem held for writing;
> + * Caller should ensure anon_name stability by raising its refcount even when
> + * anon_name belongs to a valid vma because this function might free that vma.
>   */
>  static int madvise_update_vma(struct vm_area_struct *vma,
>                               struct vm_area_struct **prev, unsigned long start,
> @@ -1021,8 +1023,10 @@ static int madvise_vma_behavior(struct vm_area_struct *vma,
>         }
>
>         anon_name = vma_anon_name(vma);
> +       anon_vma_name_get(anon_name);
>         error = madvise_update_vma(vma, prev, start, end, new_flags,
>                                    anon_name);
> +       anon_vma_name_put(anon_name);
>
>  out:
>         /*
> --
> 2.35.1.473.g83b2b277ed-goog
>