On Tue, Feb 25, 2025 at 04:54:22PM +0100, Vlastimil Babka wrote:
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.
Yeah damn, this pushes up the priority on this.
Yeah unfortunately we cannot support this with guard regions being installed _after_ the mlockall() but can for before.
Let me write this on my eternal whiteboard of doom (TM), because that ups the priority on this. I want to have a good think and see if it might after all be possible to find a way to make things work here for sake of this case.
This thing is already shipped now, so it is inevitably going to be an add-on.
I will try some experiments when I get a sec.
Thanks very much for bringing this point up! This is pretty key.
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.
Hmm yeah I suppose. I guess just to be consistent with the other _LOCKED variants? (which seem to be... undocumented at least in man pages :P, and yes I realise this is me semi-volunteering to do that obviously...).
But on the other hand, we could also expand this if you and I see also Dave feel this makes sense and wouldn't be confusing.
Agreed entirely that it'd be very very odd for a user to rely on that so I think we'll be fine.
I shall return to this topic later, in the form of a series, probably!
Cheers!