Re: [PATCH] mm: prevent droppable mappings from being locked

[anthony.yznaga@oracle.com]

On 3/9/26 7:28 AM, Lorenzo Stoakes (Oracle) wrote:
> -cc old mail (this is going to take some time to propagate I realise :P)
>
> On Fri, Mar 06, 2026 at 12:45:50PM -0800, Anthony Yznaga wrote:
>> Mappings created with MAP_DROPPABLE cannot be locked via mlock() due
>> to the check in mlock_fixup(). However, they will be locked indirectly
>> if they are created after mlockall(MCL_FUTURE).
> You need to add more details here.
>
> For e.g.: 'in apply_mlockall_flags(), if the flags parameter has MCL_FUTURE set,
> the current task's mm's default VMA flag field mm->def_flags has VM_LOCKED
> applied to it. Therefore, in __mmap_complete(), extend the test for VM_SPECIAL
> to include a test for VM_DROPPABLE'.
>
> Do you have a test that can check for this? It'd be good to have a regression
> test to assert that it now behaves correctly.
>
> You could extend either tools/testing/selftests/mm/mlock2-tests.c or
> droppable.c?
>
> It's worth mentioning that mlockall(MCL_ONFAULT) is handled too, as
> VM_LOCKONFAULT is always set with VM_LOCKED (the only difference being that,
> when trying to fault in memory for VM_LOCKED ranges, gup exits early in
> populate_vma_page_range() which has an explicit test for VM_LOCKONFAULT) , and
> apply_mlockall_flags() will invoke mlock_fixup() which already has the
> VM_DROPPABLE check.

I'll add a more detail commit message, and look at adding a test.


>
>> Fixes: 9651fcedf7b9 ("mm: add MAP_DROPPABLE for designating always lazily freeable mappings")
> Do we want to cc: stable here?

I don't think so? It seems unlikely to be hit in practice, but I 
couldn't say for sure.

Anthony

>
>> Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
>> ---
>>   include/linux/mm.h | 3 +++
>>   mm/mlock.c         | 4 ++--
>>   mm/vma.c           | 2 +-
>>   3 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 5be3d8a8f806..bb830574d112 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -574,6 +574,9 @@ enum {
>>   /* This mask represents all the VMA flag bits used by mlock */
>>   #define VM_LOCKED_MASK	(VM_LOCKED | VM_LOCKONFAULT)
>>
>> +/* This mask prevents VMAs from being mlock'd */
>> +#define VM_NO_MLOCK_MASK	(VM_SPECIAL | VM_DROPPABLE)
>> +
> It'd be preferable to not use the legacy VMA flags implementation, but if we're
> backporting I guess... However there's only one place you need to update, the
> other already manually checks droppable, and it'd make my life easier for the
> VMA flags conversions to not define a flag like this also :)
>
>>   /* These flags can be updated atomically via VMA/mmap read lock. */
>>   #define VM_ATOMIC_SET_ALLOWED VM_MAYBE_GUARD
>>
>> diff --git a/mm/mlock.c b/mm/mlock.c
>> index 2f699c3497a5..fd35c1e88c4c 100644
>> --- a/mm/mlock.c
>> +++ b/mm/mlock.c
>> @@ -472,9 +472,9 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
>>   	int ret = 0;
>>   	vm_flags_t oldflags = vma->vm_flags;
>>
>> -	if (newflags == oldflags || (oldflags & VM_SPECIAL) ||
>> +	if (newflags == oldflags || (oldflags & VM_NO_MLOCK_MASK) ||
>>   	    is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm) ||
>> -	    vma_is_dax(vma) || vma_is_secretmem(vma) || (oldflags & VM_DROPPABLE))
>> +	    vma_is_dax(vma) || vma_is_secretmem(vma))
> This obviously wouldn't be necessary without adding a new VM_xxx...
>
>>   		/* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
>>   		goto out;
>>
>> diff --git a/mm/vma.c b/mm/vma.c
>> index be64f781a3aa..1334622e4a03 100644
>> --- a/mm/vma.c
>> +++ b/mm/vma.c
>> @@ -2589,7 +2589,7 @@ static void __mmap_complete(struct mmap_state *map, struct vm_area_struct *vma)
>>
>>   	vm_stat_account(mm, vma->vm_flags, map->pglen);
>>   	if (vm_flags & VM_LOCKED) {
>> -		if ((vm_flags & VM_SPECIAL) || vma_is_dax(vma) ||
>> +		if ((vm_flags & VM_NO_MLOCK_MASK) || vma_is_dax(vma) ||
> For backport maybe just put an additional vm_flags & VM_DROPPABLE here?
>
>>   					is_vm_hugetlb_page(vma) ||
>>   					vma == get_gate_vma(mm))
>>   			vm_flags_clear(vma, VM_LOCKED_MASK);
>> --
>> 2.47.3
>>
> Though I saw David suggested something different so that also addresses my review here :)
>
> Cheers, Lorenzo