From: Suren Baghdasaryan <surenb@google.com>
To: Michal Hocko <mhocko@suse.com>
Cc: akpm@linux-foundation.org, ccross@google.com,
sumit.semwal@linaro.org, dave.hansen@intel.com,
keescook@chromium.org, willy@infradead.org,
kirill.shutemov@linux.intel.com, vbabka@suse.cz,
hannes@cmpxchg.org, ebiederm@xmission.com, brauner@kernel.org,
legion@kernel.org, ran.xiaokai@zte.com.cn, sashal@kernel.org,
chris.hyser@oracle.com, dave@stgolabs.net, pcc@google.com,
caoxiaofeng@yulong.com, david@redhat.com, gorcunov@gmail.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
kernel-team@android.com,
syzbot+aa7b3d4b35f9dc46a366@syzkaller.appspotmail.com
Subject: Re: [PATCH v4 3/3] mm: fix use-after-free when anon vma name is used after vma is freed
Date: Tue, 22 Feb 2022 07:43:40 -0800 [thread overview]
Message-ID: <CAJuCfpELxJ=7uuurKL9oRn1E_=rfL3aN8Duhqvi4Z2c1xHAT2w@mail.gmail.com> (raw)
In-Reply-To: <YhSZhzUYlW6IAQT9@dhcp22.suse.cz>
On Tue, Feb 22, 2022 at 12:06 AM Michal Hocko <mhocko@suse.com> wrote:
>
> On Mon 21-02-22 21:40:25, Suren Baghdasaryan 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
>
> This seems to be stale because bare const char pointer is not passed in
> the call chain. In fact I am not even sure there is any actual UAF here
> after the rework.
> Could you be more specific in describing the scenario?
Yes, sorry, I need to update the part of the description talking about
passing vma->anon_name->name directly.
I think UAF is still there, it's just harder to reproduce (admittedly
I could not reproduce it with the previous reproducer). The scenario
would be when a vma with vma->anon_name->kref == 1 is being merged
with another one and freed in the process:
madvise_vma_behavior
anon_name = vma_anon_name(vma) <-- does not increase refcount
madvise_update_vma(anon_name)
*prev = vma_merge <-- returns another vma
__vma_adjust
vm_area_free(vma)
free_vma_anon_name
anon_vma_name_put
vma_anon_name_free <-- frees the vma->anon_name
vma = *prev <-- original vma was freed
replace_vma_anon_name(vma, >>anon_name<<) <-- UAF
Does this make sense or did I miss something?
Thanks,
Suren.
>
> > 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@google.com>
> > Reported-by: syzbot+aa7b3d4b35f9dc46a366@syzkaller.appspotmail.com
> > ---
> > changes in v3:
> > - Reapplied the fix after code refactoring, per Michal Hocko
> >
> > 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
>
> --
> Michal Hocko
> SUSE Labs
next prev parent reply other threads:[~2022-02-22 15:43 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-22 5:40 [PATCH 1/3] mm: refactor vm_area_struct::anon_vma_name usage code Suren Baghdasaryan
2022-02-22 5:40 ` [PATCH 2/3] mm: prevent vm_area_struct::anon_name refcount saturation Suren Baghdasaryan
2022-02-22 9:17 ` Michal Hocko
2022-02-22 15:56 ` Suren Baghdasaryan
2022-02-23 3:02 ` Suren Baghdasaryan
2022-02-23 8:26 ` Michal Hocko
2022-02-22 5:40 ` [PATCH v4 3/3] mm: fix use-after-free when anon vma name is used after vma is freed Suren Baghdasaryan
2022-02-22 5:41 ` Suren Baghdasaryan
2022-02-22 8:06 ` Michal Hocko
2022-02-22 15:43 ` Suren Baghdasaryan [this message]
2022-02-23 8:55 ` Michal Hocko
2022-02-23 14:10 ` Suren Baghdasaryan
2022-02-23 15:29 ` Michal Hocko
2022-02-23 15:38 ` Suren Baghdasaryan
2022-02-22 8:51 ` [PATCH 1/3] mm: refactor vm_area_struct::anon_vma_name usage code Michal Hocko
2022-02-22 15:51 ` Suren Baghdasaryan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJuCfpELxJ=7uuurKL9oRn1E_=rfL3aN8Duhqvi4Z2c1xHAT2w@mail.gmail.com' \
--to=surenb@google.com \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=caoxiaofeng@yulong.com \
--cc=ccross@google.com \
--cc=chris.hyser@oracle.com \
--cc=dave.hansen@intel.com \
--cc=dave@stgolabs.net \
--cc=david@redhat.com \
--cc=ebiederm@xmission.com \
--cc=gorcunov@gmail.com \
--cc=hannes@cmpxchg.org \
--cc=keescook@chromium.org \
--cc=kernel-team@android.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=legion@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=pcc@google.com \
--cc=ran.xiaokai@zte.com.cn \
--cc=sashal@kernel.org \
--cc=sumit.semwal@linaro.org \
--cc=syzbot+aa7b3d4b35f9dc46a366@syzkaller.appspotmail.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.