Lorenzo Stoakes <[email protected]> wrote:
> This patch updates secretmem to use the new vma_flags_t type which will
> soon supersede vm_flags_t altogether.
>
> In order to make this change we also have to update mlock_future_ok(), we
> replace the vm_flags_t parameter with a simple boolean is_vma_locked one,
> which also simplifies the invocation here.
>
> This is laying the groundwork for eliminating the vm_flags_t in
> vm_area_desc and more broadly throughout the kernel.
>
Hi everyone,
Running mm-new through AI review flagged this one:
> diff --git a/mm/mmap.c b/mm/mmap.c
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -225,12 +225,12 @@ static inline unsigned long round_hint_to_min(unsigned
> long hint)
> return hint;
> }
>
> -bool mlock_future_ok(const struct mm_struct *mm, vm_flags_t vm_flags,
> - unsigned long bytes)
> +bool mlock_future_ok(const struct mm_struct *mm, bool is_vma_locked,
> + unsigned long bytes)
> {
> unsigned long locked_pages, limit_pages;
>
> - if (!(vm_flags & VM_LOCKED) || capable(CAP_IPC_LOCK))
> + if (!is_vma_locked || capable(CAP_IPC_LOCK))
> return true;
[ ... ]
> @@ -416,7 +416,7 @@
> if (!can_do_mlock())
> return -EPERM;
>
> - if (!mlock_future_ok(mm, vm_flags, len))
> + if (!mlock_future_ok(mm, vm_flags & VM_LOCKED, len))
> return -EAGAIN;
This looks correct - do_mmap() now extracts the VM_LOCKED bit from
vm_flags and passes that to mlock_future_ok().
However, it appears check_brk_limits() was not updated when
mlock_future_ok() changed its signature. The function at mm/mmap.c:111
still passes the raw def_flags value:
return mlock_future_ok(current->mm, current->mm->def_flags, len)
? 0 : -EAGAIN;
When def_flags has any bit set, this converts to true, incorrectly
triggering mlock limit checks even when VM_LOCKED is not set. Should
this be passing (current->mm->def_flags & VM_LOCKED) instead?
[ ... ]
Additionally, the test stub at tools/testing/vma/vma_internal.h:1627
still has the old signature (mm, vm_flags_t vm_flags, bytes) while the
production code now uses (mm, bool is_vma_locked, bytes). This could
cause compilation issues or mask bugs in the test suite.
