Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows

[David Hildenbrand (Arm)]

On 9/29/26 07:56, Park Tae-sun wrote:
> mlock() and munlock() currently normalize the requested address range
> using:
> 
>     len = PAGE_ALIGN(len + offset_in_page(start));
>     start &= PAGE_MASK;
> 
> This performs multiple arithmetic operations on user-provided values
> without validating intermediate overflows.
> 
> First, a zero-length request with an unaligned address is incorrectly
> converted into a non-zero page range. Because
> PAGE_ALIGN(offset_in_page(start)) rounds any non-zero offset up to a
> full page, a request such as mlock(0x1005, 0) falsely locks an entire
> 4KB page (or fails with ENOMEM if unmapped), and munlock(0x1005, 0)
> silently unlocks memory that should remain locked. Conversely, an
> aligned mlock(0x1000, 0) is a no-op. Zero-length behavior should be a
> consistent no-op regardless of address alignment.
> 
> Second, a sufficiently large length wraps around to zero during page
> alignment (e.g., len = ULONG_MAX). Because PAGE_ALIGN(x) is defined as:
> 
>     (((x) + PAGE_SIZE - 1) & PAGE_MASK)
> 
> when len is close to ULONG_MAX, adding (PAGE_SIZE - 1) wraps around:
> on a 64-bit system with 4KB pages, ULONG_MAX + 4095 wraps to 4094, and
> masking with PAGE_MASK clears the lower 12 bits, producing 0.
> When len becomes 0, the subsequent check in apply_vma_lock_flags():
> 
>     end = start + len;
>     if (end == start)
>         return 0;
> 
> evaluates to true and immediately returns 0 (success) without locking or
> unlocking the requested memory range. Similar wrap-around issues have
> previously been addressed in mincore() and madvise() by checking for
> zero length after PAGE_ALIGN().
> 
> Finally, address addition overflow (start + len < start) is currently
> detected only inside apply_vma_lock_flags(), after mmap_write_lock has
> already been acquired and memlock rlimits checked, improperly returning
> -ENOMEM instead of -EINVAL. Per POSIX.1-2024 and man 2 mlock, arithmetic
> overflow of the requested range represents an invalid argument and must
> fail with -EINVAL before modifying VMAs or acquiring locks. Note that
> apply_vma_lock_flags() already contains:
> 
>     if (end < start)
>         return -EINVAL;
> 
> but this was obscured because rlimit accounting preceded it.
> 
> Introduce a common check_mlock_range() helper to:
> 1. Return 0 immediately for zero-length requests without taking mmap_lock.
> 2. Validate intermediate and final arithmetic additions using
>    check_add_overflow().
> 3. Reject lengths that wrap to zero under PAGE_ALIGN() with -EINVAL.
> 4. Detect start + len address overflow before taking mmap_write_lock.
> 
> Signed-off-by: Park Tae-sun <ts930@xxxxxxxxx>
> ---
>  mm/mlock.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 47 insertions(+), 8 deletions(-)

You call it "fix" but then I see no Fixes: tag.

Which of the problems described above have reproducers (IOW can be triggered)
and what is the user-visible problem?

-- 
Cheers,

David