linux-kernel - Re: [PATCH] mm: Update do_vmi_align

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] [day] [month] [year] [list]

Date:   Fri, 30 Jun 2023 15:07:08 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] mm: Update do_vmi_align_munmap() return semantics

* Linus Torvalds <torvalds@...ux-foundation.org> [230630 13:21]:
> On Fri, 30 Jun 2023 at 09:41, Liam R. Howlett <Liam.Howlett@...cle.com> wrote:
> >
> > I hesitate to ask considering how much trouble I've caused with the
> > 32bit map flag, but I also wonder about the stack guard now that the
> > write lock is taken for stack expansion?
> 
> Which part of it? We have a lot of special code wrt the whole thing
> that came from how we did the expansion that probably can - and should
> - be cleaned up.
> 
> For example I didn't want to go remove our ad-hoc locking, so we still
> do that "mm->page_table_lock" thing.
> 
> And I think the stack expansion does several things differently from
> the "normal" vma games in general, because it explicitly didn't want
> to use the normal "merge vma" code because it didn't do real locking.

I think this is probably rare enough to not be worth the added
complexity?

> 
> But you're talking about the general issue of having a stack guard
> area at all, _that_ isn't affected by the locking.

I was thinking about the searching for free area, which falls into this
category so the locking change doesn't help much to clean this up.

> 
> That was always a real semantic issue of "we don't want user space
> stack growth to possibly grow into another vma, and because the stack
> growing isn't strictly predictable, we need to have that guard area in
> between to catch things when they get too close".
> 
> So the stack guard isn't there to protect stack vma's from merging.
> It's there to protect users from mistakes.
> 
> And then we have all those very rare situations where we *do* want
> stacks to merge, and the guard goes away, but we currently do *not*
> call vma_merge(), and just leave it as two adjacent vma's because we
> used to only have a read-lock.
> 
> End result: I do think that doing the locking right means that we may
> be able to clean up some other code. The odd do_vmi_align_munmap()
> case is just one of the more egregious special cases.

Thanks for the insight.

Anywhere that asks for a downgrade can be changed to an unlock so here's
v2 that makes that change as well.

Thanks,
Liam

View attachment "v2-0001-mm-Update-do_vmi_align_munmap-return-semantics.patch" of type "text/x-diff" (12397 bytes)