linux-kernel - Re: [PATCH v2 3/5] mm: madvise: implement lightweight guard page mechanism

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: <efc2af8d-6e6f-4bd2-9edf-673ea622185c@lucifer.local>
Date: Mon, 21 Oct 2024 22:46:11 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: David Hildenbrand <david@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Matthew Wilcox <willy@...radead.org>,
        "Paul E . McKenney" <paulmck@...nel.org>, Jann Horn <jannh@...gle.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Muchun Song <muchun.song@...ux.dev>,
        Richard Henderson <richard.henderson@...aro.org>,
        Ivan Kokshaysky <ink@...assic.park.msu.ru>,
        Matt Turner <mattst88@...il.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        "James E . J . Bottomley" <James.Bottomley@...senpartnership.com>,
        Helge Deller <deller@....de>, Chris Zankel <chris@...kel.net>,
        Max Filippov <jcmvbkbc@...il.com>, Arnd Bergmann <arnd@...db.de>,
        linux-alpha@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-parisc@...r.kernel.org, linux-arch@...r.kernel.org,
        Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>,
        linux-kselftest@...r.kernel.org,
        Sidhartha Kumar <sidhartha.kumar@...cle.com>,
        Jeff Xu <jeffxu@...omium.org>, Christoph Hellwig <hch@...radead.org>,
        linux-api@...r.kernel.org, John Hubbard <jhubbard@...dia.com>
Subject: Re: [PATCH v2 3/5] mm: madvise: implement lightweight guard page
 mechanism

On Mon, Oct 21, 2024 at 11:35:24PM +0200, Vlastimil Babka wrote:
> On 10/21/24 23:20, David Hildenbrand wrote:
> >> I don't think there's really any value in that. There's just no sensible
> >> situation in which a user would care about this I don't think.
> >
> > Making sure nobody touches an area, and wile doing that somebody already
> > touched that area? I guess it could be worked around by
> > mprotect(PROT_NONE),madvise(GUARD),mprotect(PROT_READ|PROT_WRITE) ...
> > which is not particularly nice :)
> >
> >>
> >> And if you're saying 'hey do MADV_DONTNEED if this fails and keep trying!'
> >> then why not just do that in the kernel?
> >
> > Heh, no!
> >
> > If user space doesn't expect there to be something, it should *fail*.
> > That's likely going to be the majority of use cases for guard pages
> > (happy to be told otherwise). No retry.
> >
> > And if user space expects there to be something it should zap ahead of
> > time (which some allocators maybe already do to free up memory after
> > free()) to then install the guard. No retry.
> >
> > There is this case where user space might be unsure. There, it might
> > make sense to retry exactly once.
>
> I've thought so too and the RFC was implemented like this, but Jann came up
> with a scenario where a THP can cause the range including our
> to-be-installed guard pte to be populated even if the userspace is not
> trying to access that exact address, see here:
>
> https://lore.kernel.org/all/CAG48ez3vqbqyWb4bLdpqSUnhwqGo2OQetecNhEGPdCGDr94nbQ@mail.gmail.com/
>
> So unless we can't *reliably* detect that userspace is really shooting
> itself in the foot and return a failure to install guard pte *only* in that
> case (which would be useful), and not horribly complicate everything to
> ensure that reliability and to avoid false positives due to races with
> THP's, then it's probably better to just retry as this version does.

It would be complicated, and I'd reallly like to avoid trying to detect
this. It feels a bit whack-a-mole because maybe there's other scenarios
we've not thought about that could be equally problematic?

>
> >>
> >> Trying to explain to a user 'hey this is for installing guard pages but if
> >> there's a facing fault it'll fail and that could keep happening and then
> >> you'll have to zap and maybe in a loop' just... seems like a bloody awful
> >> interface?
> >
> > Hope my example above made it clearer. This "retry forever until it
> > works" use case doesn't quite make sense to me, but I might just be
> > missing something important.
> >
> > But again, I have to do more reading on the history of the current
> > approach ... and it's fairly late here.
>
> Yeah see the RFC thread linked above.
>

Right, but I don't think this is the only scenario that can happen, and I
think, FWIW, yet again the fundamental point comes down to 'is it a
problem?'

Because if looping like this isn't, then problem solved we can all high 5
and go home listening to the prodigy and full happiness.

If not then we can revisit.

And how could it be a problem? Surely only security or DoS
potential. Hopefully Jann can give some positive feedback on that.

We could also, and I hate to say it, but... try to find some means of
checking on reasonable forward progress in the loop if we had to or some
other 'reasonable attempt'.

But let's see...