linux-kernel - Re: [PATCH] mm: Fix MREMAP_DONTUNMAP accounting on VMA merge

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: <CAHk-=wgOYDi2pqECSGnxnBijemqWoLPWDh1LSKV5UrSivUUoRA@mail.gmail.com>
Date:   Sun, 19 Apr 2020 10:59:59 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Brian Geffon <bgeffon@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...capital.net>,
        Sonny Rao <sonnyrao@...gle.com>,
        Jesse Barnes <jsbarnes@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Minchan Kim <minchan@...nel.org>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        Vlastimil Babka <vbabka@...e.cz>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>,
        Linux API <linux-api@...r.kernel.org>,
        syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH] mm: Fix MREMAP_DONTUNMAP accounting on VMA merge

On Fri, Apr 17, 2020 at 10:26 AM Brian Geffon <bgeffon@...gle.com> wrote:
>
> However, MREMAP_DONTUNMAP leaves that original portion in place which
> means that the VMA which was split and then remerged is not actually
> split at the end of the mremap.

I was waiting to hear others comment on this, but it's been very quiet.

The patch looks correct to me, and the explanation is great. I'm
inclined to just apply it.

HOWEVER.

I started looking at copy_vma(), and noticed that we seem to have
exactly one caller: move_vma().

So I do have a query: would it perhaps not be a good idea to simply
remove the "vma_merge()" call from copy_vma(), and do at the end of
move_vma() instead?

I don't hate this patch either, and I'll happily apply it if people
prefer this one, but before doing that I thought I'd ask whether maybe
instead of fixing up the mess made by vma_merge() that people didn't
think about, maybe we should fix it at the underlying source of the
problem?

Are there any advantages to merging early? Shouldn't the basic
principle be that we'd strive to always do the vma_merge() at the end
of an operation that might have generated a mergable sequence of
vma's?

               Linus