linux-kernel - Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check

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: <16e8ac61-d0ec-43aa-8467-17a3c2ea5962@lucifer.local>
Date: Mon, 14 Jul 2025 16:32:47 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: Pedro Falcato <pfalcato@...e.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Jeff Xu <jeffxu@...omium.org>
Subject: Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check

On Mon, Jul 14, 2025 at 05:25:59PM +0200, David Hildenbrand wrote:
> On 14.07.25 17:23, Lorenzo Stoakes wrote:
> > On Mon, Jul 14, 2025 at 04:17:23PM +0100, Pedro Falcato wrote:
> > > On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
> > > > The check_mm_seal() function is doing something general - checking whether
> > > > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > > > regions).
> > > >
> > > > Generalise this and put the logic in mm/vma.c - introducing
> > > > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > > > simply checking whether the last vma->vm_end has either a VMA starting
> > > > after it or ends before the end parameter.
> > > >
> > >
> > > I don't like this. Unless you have any other user for this in mind,
> > > we'll proliferate this awful behavior (and add this into core vma code).
> >
> > I'm not sure how putting it in an internal-only mm file perpetuates
> > anything.
> >
> > I'm naming the function by what it does, and putting it where it belongs in
> > the VMA logic, and additionally making the function less horrible.
> >
> > Let's not please get stuck on the isues with mseal implementation which
> > will catch-22 us into not being able to refactor.
> >
> > We can do the refactoring first and it's fine to just yank this if it's not
> > used.
> >
> > I'm not having a function like this sat in mm/mseal.c when it has
> > absolutely nothing to do with mseal specifically though.
> >
> > >
> > > I have some patches locally to fully remove this upfront check, and AFAIK
> > > we're somewhat in agreement that we can simply nuke this check (for
> > > various reasons, including that we *still* don't have a man page for the
> > > syscall). I can send them for proper discussion after your series lands.
> >
> > Yes I agree this check is odd, I don't really see why on earth we're
> > concerned with whether there are gaps or not, you'd surely want to just
> > seal whatever VMAs exist in the range?
>
> Probably because GAPs cannot be sealed. So user space could assume that in
> fact, nothing in that area can change after a successful mseal, while it can
> ...
>
> Not sure, though ...

Yeah maybe a sekuriteh thing where you want to be sure the range is in fact
_all_ sealed.

I'm not sure that really makes much sense in practice honestly though, because
if another thread can fiddle with things, then surely you've already 'lost'.

if you expected to touch a VMA where in fact aa gap exists your software is
_already_ broken because you're going to segfault.

So it just seems overly theoretical to me.

I think we should error out if there's _no_ VMAs at all, but otherwise succeed.

The semantics of 'all VMAs which exist in the range are sealed' would still be
maintained.

>
> --
> Cheers,
>
> David / dhildenb
>