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

[Lorenzo Stoakes (ARM)]

On Tue, Sep 29, 2026 at 10:40:13AM +0200, David Hildenbrand (Arm) wrote:
> 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.

Or an Assisted-by tag :)

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

Good question!

Also note that the code is just horrible - 2 output parameters into a function
called 'check' but mutates all of its input, deref of the input parameters
throughout the function, useless comments, etc.

This patch is not upstreamable, and if it's LLM-generated I'd rather that
somebody from the core team did the work if we wanted it.

In general, I think it might be helpful to have a general 'check for overflow
stuff' function that various syscalls could use, but I'd want to see that
developed by an experienced mm person.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo