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

[David Hildenbrand <david@redhat.com>]

On 21.10.24 22:11, Vlastimil Babka wrote:
> On 10/20/24 18:20, Lorenzo Stoakes wrote:
>> Implement a new lightweight guard page feature, that is regions of userland
>> virtual memory that, when accessed, cause a fatal signal to arise.
>>
>> Currently users must establish PROT_NONE ranges to achieve this.
>>
>> However this is very costly memory-wise - we need a VMA for each and every
>> one of these regions AND they become unmergeable with surrounding VMAs.
>>
>> In addition repeated mmap() calls require repeated kernel context switches
>> and contention of the mmap lock to install these ranges, potentially also
>> having to unmap memory if installed over existing ranges.
>>
>> The lightweight guard approach eliminates the VMA cost altogether - rather
>> than establishing a PROT_NONE VMA, it operates at the level of page table
>> entries - poisoning PTEs such that accesses to them cause a fault followed
>> by a SIGSGEV signal being raised.
>>
>> This is achieved through the PTE marker mechanism, which a previous commit
>> in this series extended to permit this to be done, installed via the
>> generic page walking logic, also extended by a prior commit for this
>> purpose.
>>
>> These poison ranges are established with MADV_GUARD_POISON, and if the
>> range in which they are installed contain any existing mappings, they will
>> be zapped, i.e. free the range and unmap memory (thus mimicking the
>> behaviour of MADV_DONTNEED in this respect).
>>
>> Any existing poison entries will be left untouched. There is no nesting of
>> poisoned pages.
>>
>> Poisoned ranges are NOT cleared by MADV_DONTNEED, as this would be rather
>> unexpected behaviour, but are cleared on process teardown or unmapping of
>> memory ranges.
>>
>> Ranges can have the poison property removed by MADV_GUARD_UNPOISON -
>> 'remedying' the poisoning. The ranges over which this is applied, should
>> they contain non-poison entries, will be untouched, only poison entries
>> will be cleared.
>>
>> We permit this operation on anonymous memory only, and only VMAs which are
>> non-special, non-huge and not mlock()'d (if we permitted this we'd have to
>> drop locked pages which would be rather counterintuitive).
>>
>> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
>> Suggested-by: Jann Horn <jannh@google.com>
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
> <snip>
> 
>> +static long madvise_guard_poison(struct vm_area_struct *vma,
>> +				 struct vm_area_struct **prev,
>> +				 unsigned long start, unsigned long end)
>> +{
>> +	long err;
>> +
>> +	*prev = vma;
>> +	if (!is_valid_guard_vma(vma, /* allow_locked = */false))
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * If we install poison markers, then the range is no longer
>> +	 * empty from a page table perspective and therefore it's
>> +	 * appropriate to have an anon_vma.
>> +	 *
>> +	 * This ensures that on fork, we copy page tables correctly.
>> +	 */
>> +	err = anon_vma_prepare(vma);
>> +	if (err)
>> +		return err;
>> +
>> +	/*
>> +	 * Optimistically try to install the guard poison pages first. If any
>> +	 * non-guard pages are encountered, give up and zap the range before
>> +	 * trying again.
>> +	 */
> 
> Should the page walker become powerful enough to handle this in one go? :)
> But sure, if it's too big a task to teach it to zap ptes with all the tlb
> flushing etc (I assume it's something page walkers don't do today), it makes
> sense to do it this way.
> Or we could require userspace to zap first (MADV_DONTNEED), but that would
> unnecessarily mean extra syscalls for the use case of an allocator debug
> mode that wants to turn freed memory to guards to catch use after free.
> So this seems like a good compromise...

Yes please, KIS. We can always implement support for that later if 
really required (leave behavior open when documenting).

-- 
Cheers,

David / dhildenb