[mm] VMA merging behavior wrt anon_vma has been slightly broken since Linux 3.0 (in a non-dangerous way)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux