From: Jann Horn <jannh@google.com>
To: Linux-MM <linux-mm@kvack.org>,
Andrew Morton <akpm@linux-foundation.org>,
Shaohua Li <shaohua.li@intel.com>
Cc: kernel list <linux-kernel@vger.kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Liam Howlett <liam.howlett@oracle.com>
Subject: [mm] VMA merging behavior wrt anon_vma has been slightly broken since Linux 3.0 (in a non-dangerous way)
Date: Tue, 15 Aug 2023 21:43:46 +0200 [thread overview]
Message-ID: <CAG48ez1KYR9pY1s0=9QH_n5cY-_Zejajj2JEa-se=UZQbeN4hw@mail.gmail.com> (raw)
Hi!
I think VMA merging was accidentally nerfed a bit by commit
965f55dea0e3 ("mmap: avoid merging cloned VMAs"), which landed in
Linux 3.0 - essentially, that commit makes it impossible to merge a
VMA with an anon_vma into an adjacent VMA that does not have an
anon_vma. (But the other direction works.)
is_mergeable_anon_vma() is defined as:
```
static inline bool is_mergeable_anon_vma(struct anon_vma *anon_vma1,
struct anon_vma *anon_vma2, struct vm_area_struct *vma)
{
/*
* The list_is_singular() test is to avoid merging VMA cloned from
* parents. This can improve scalability caused by anon_vma lock.
*/
if ((!anon_vma1 || !anon_vma2) && (!vma ||
list_is_singular(&vma->anon_vma_chain)))
return true;
return anon_vma1 == anon_vma2;
}
```
If this function is called with a non-NULL vma pointer (which is
almost always the case, except when checking for whether it's possible
to merge in both directions at the same time), and one of the two
anon_vmas is non-NULL, this returns
list_is_singular(&vma->anon_vma_chain). I believe that
list_is_singular() call is supposed to check whether the
anon_vma_chain contains *more than one* element, but it actually also
fails if the anon_vma_chain contains zero elements.
This means that the dup_anon_vma() calls in vma_merge() are
effectively all no-ops because they are never called with a target
that does not have an anon_vma and a source that has an anon_vma.
I think this is unintentional - though I guess this unintentional
refusal to merge VMAs this way also lowers the complexity of what can
happen in the VMA merging logic. So I think the right fix here is to
make this kind of merging possible again by changing
"list_is_singular(&vma->anon_vma_chain)" to
"list_empty(&vma->anon_vma_chain) ||
list_is_singular(&vma->anon_vma_chain)", but my security hat makes me
say that I'd also be happy if the unintentional breakage stayed this
way it is now.
next reply other threads:[~2023-08-15 19:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-15 19:43 Jann Horn [this message]
2023-08-17 13:42 ` Liam R. Howlett
2023-08-17 16:21 ` Jann Horn
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='CAG48ez1KYR9pY1s0=9QH_n5cY-_Zejajj2JEa-se=UZQbeN4hw@mail.gmail.com' \
--to=jannh@google.com \
--cc=akpm@linux-foundation.org \
--cc=liam.howlett@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=peterz@infradead.org \
--cc=shaohua.li@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox