[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e0954e13-2c7d-447c-ba86-19875c74bc3b@suse.cz>
Date: Tue, 25 Feb 2025 16:54:22 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
David Hildenbrand <david@...hat.com>
Cc: 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, Shuah Khan <shuah@...nel.org>,
linux-kselftest@...r.kernel.org, linux-api@...r.kernel.org,
John Hubbard <jhubbard@...dia.com>, Juan Yescas <jyescas@...gle.com>,
Kalesh Singh <kaleshsingh@...gle.com>
Subject: Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem
mappings
On 2/18/25 18:28, Lorenzo Stoakes wrote:
> On Tue, Feb 18, 2025 at 06:25:35PM +0100, David Hildenbrand wrote:
>>
>> > > >
>> > > > It fails because it tries to 'touch' the memory, but 'touching' guard
>> > > > region memory causes a segfault. This kind of breaks the idea of
>> > > > mlock()'ing guard regions.
>> > > >
>> > > > I think adding workarounds to make this possible in any way is not really
>> > > > worth it (and would probably be pretty gross).
>> > > >
>> > > > We already document that 'mlock()ing lightweight guard regions will fail'
>> > > > as per man page so this is all in line with that.
>> > >
>> > > Right, and I claim that supporting VM_LOCKONFAULT might likely be as easy as
>> > > allowing install/remove of guard regions when that flag is set.
>> >
>> > We already allow this flag! VM_LOCKED and VM_HUGETLB are the only flags we
>> > disallow.
>>
>>
>> See mlock2();
>>
>> SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
>> {
>> vm_flags_t vm_flags = VM_LOCKED;
>>
>> if (flags & ~MLOCK_ONFAULT)
>> return -EINVAL;
>>
>> if (flags & MLOCK_ONFAULT)
>> vm_flags |= VM_LOCKONFAULT;
>>
>> return do_mlock(start, len, vm_flags);
>> }
>>
>>
>> VM_LOCKONFAULT always as VM_LOCKED set as well.
>
> OK cool, that makes sense.
>
> As with much kernel stuff, I knew this in the past. Then I forgot. Then I knew
> again, then... :P if only somebody would write it down in a book...
>
> Yeah then that makes sense to check explicitly for (VM_LOCKED | VM_LOCKONFAULT)
> in any MADV_GUARD_INSTALL_LOCKED variant as obviously this would be passively
> excluded right now.
Sorry for the late reply. So AFAIU from your conversations, guards can't be
compatible with VM_LOCKED, which means e.g. any attempts of glibc to use
guards for stacks will soon discover that mlockall() users exist and are
broken by this, and the attempts will fail? That's a bummer.
As for compatibility with VM_LOCKONFAULT, do we need a new
MADV_GUARD_INSTALL_LOCKED or can we say MADV_GUARD_INSTALL is new enough
that it can be just retrofitted (like you retrofit file backed mappings)?
AFAIU the only risk would be breaking somebody that already relies on a
failure for VM_LOCKONFAULT, and it's unlikely there's such a somebody now.
Powered by blists - more mailing lists