mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
@ 2026-09-29  5:56 Park Tae-sun
  2026-09-29  8:30 ` Jose A. Perez de Azpillaga
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Park Tae-sun @ 2026-09-29  5:56 UTC (permalink / raw)
  To: Andrew Morton, Liam R. Howlett, Lorenzo Stoakes
  Cc: David Hildenbrand, Vlastimil Babka, Jann Horn, Pedro Falcato,
	Michal Hocko, Mike Rapoport, Suren Baghdasaryan, linux-mm,
	linux-kernel, Park Tae-sun

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@dgu.ac.kr>
---
 mm/mlock.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/mm/mlock.c b/mm/mlock.c
index 39215a3eab1f..203017797076 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -616,6 +616,47 @@ static int __mlock_posix_error_return(long retval)
 	return retval;
 }
 
+/**
+ * check_mlock_range - Validate and page-align the requested address range.
+ * @start: Pointer to the start address. Untagged and page-aligned on success.
+ * @len:   Pointer to the length. Page-aligned on success.
+ *
+ * Return: 0 if the range is valid, or -EINVAL on arithmetic overflow.
+ * If *len is 0, *len remains 0 and 0 is returned; callers should treat this
+ * as a no-op and return 0 immediately without acquiring mmap_lock.
+ */
+static int check_mlock_range(unsigned long *start, size_t *len)
+{
+	unsigned long end;
+
+	/* Untag user pointer (e.g. AArch64 TBI / x86 LAM) before arithmetic. */
+	*start = untagged_addr(*start);
+
+	/*
+	 * A zero-length request must not be rounded up by PAGE_ALIGN()
+	 * to a non-zero range when *start is unaligned. Return 0 so caller
+	 * can exit early without modifying VMAs or taking locks.
+	 */
+	if (!*len)
+		return 0;
+
+	/* Guard against addition overflow between length and start offset. */
+	if (check_add_overflow(*len, (size_t)offset_in_page(*start), len))
+		return -EINVAL;
+
+	*len = PAGE_ALIGN(*len);
+	/* Check whether PAGE_ALIGN() wrapped a non-zero length to zero. */
+	if (!*len)
+		return -EINVAL;
+
+	*start &= PAGE_MASK;
+	/* Reject address wrap-around (start + len < start) before taking locks. */
+	if (check_add_overflow(*start, (unsigned long)*len, &end))
+		return -EINVAL;
+
+	return 0;
+}
+
 static __must_check int do_mlock(unsigned long start, size_t len,
 				 vma_flags_t *flags)
 {
@@ -623,13 +664,12 @@ static __must_check int do_mlock(unsigned long start, size_t len,
 	unsigned long lock_limit;
 	int error = -ENOMEM;
 
-	start = untagged_addr(start);
-
 	if (!can_do_mlock())
 		return -EPERM;
 
-	len = PAGE_ALIGN(len + (offset_in_page(start)));
-	start &= PAGE_MASK;
+	error = check_mlock_range(&start, &len);
+	if (error || !len)
+		return error;
 
 	lock_limit = rlimit(RLIMIT_MEMLOCK);
 	lock_limit >>= PAGE_SHIFT;
@@ -689,10 +729,9 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
 	vma_flags_t flags = EMPTY_VMA_FLAGS;
 	int ret;
 
-	start = untagged_addr(start);
-
-	len = PAGE_ALIGN(len + (offset_in_page(start)));
-	start &= PAGE_MASK;
+	ret = check_mlock_range(&start, &len);
+	if (ret || !len)
+		return ret;
 
 	if (mmap_write_lock_killable(current->mm))
 		return -EINTR;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
  2026-09-29  5:56 [PATCH] mm/mlock: fix zero-length request normalization and integer overflows Park Tae-sun
@ 2026-09-29  8:30 ` Jose A. Perez de Azpillaga
  2026-09-29  8:38 ` Lorenzo Stoakes (ARM)
  2026-09-29  8:40 ` David Hildenbrand (Arm)
  2 siblings, 0 replies; 6+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-09-29  8:30 UTC (permalink / raw)
  To: Park Tae-sun
  Cc: Andrew Morton, Liam R. Howlett, Lorenzo Stoakes,
	David Hildenbrand, Vlastimil Babka, Jann Horn, Pedro Falcato,
	Michal Hocko, Mike Rapoport, Suren Baghdasaryan, linux-mm,
	linux-kernel

On Tue, Sep 29, 2026 at 02:56:34PM +0900, 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.

I think can_do_mlock() runs before the early return, so mlock(addr, 0)
still returns EPERM in the RLIMIT_MEMLOCK=0 case without CAP_IPC_LOCK,
while munlock(addr, 0) returns 0. should the zero length check move above
it?

moving it up also changes the aligned case, mlock(0x1000, 0) goes from
EPERM to 0 for such a task, so that is a uapi change and the changelog
should say so.

> 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().

I could not find that check in mincore. as far as I can see it avoids
PAGE_ALIGN rather than looking at the result for zero. madvise's
check_input_range() in mm/madvise.c looks like the one that does what you
describe, and there a zero length is a no op. am I reading that wrong?

> 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.

I think the errno also depends on the capability today, ENOMEM without
CAP_IPC_LOCK and EINVAL with it, so maybe the changelog should say that.

I also wonder if the wrap around part should be its own patch. the
silent success goes back to at least 2.6.12, so there is no commit to
point a Fixes tag at, but it looks like a bug, and the zero length
change is a separate behavior change that should not ride along with
it. a Cc stable would be reasonable for that part. if there is a POSIX
reference saying zero length is a valid no op it would help, or a
statement that POSIX leaves it unspecified, since EINVAL would be the
other defensible answer.

a test would help for that, len 0 with aligned and unaligned start,
ULONG_MAX, and a start plus len overflow, with and without CAP_IPC_LOCK,
without it the errno matrix can regress quietly.

the rest of the helper looked fine to me.

> 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.

--
cheers,
jose a. p-a

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
  2026-09-29  5:56 [PATCH] mm/mlock: fix zero-length request normalization and integer overflows Park Tae-sun
  2026-09-29  8:30 ` Jose A. Perez de Azpillaga
@ 2026-09-29  8:38 ` Lorenzo Stoakes (ARM)
  2026-09-29  8:40 ` David Hildenbrand (Arm)
  2 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-29  8:38 UTC (permalink / raw)
  To: Park Tae-sun
  Cc: Andrew Morton, Liam R. Howlett, David Hildenbrand,
	Vlastimil Babka, Jann Horn, Pedro Falcato, Michal Hocko,
	Mike Rapoport, Suren Baghdasaryan, linux-mm, linux-kernel

Hi,

For one of several possible reasons this mail has triggered an AI
detector script.

Note that, while we are fine with AI assistance, it is kernel
policy that you must disclose this with an Assisted-by tag like:

Assisted-by: LLM

See https://docs.kernel.org/process/coding-assistants.html

It is also kernel policy that you must fully understand and take
responsibility for every patch that you send.

See https://docs.kernel.org/process/generated-content.html most
notably:

        If tools permit you to generate a contribution automatically, expect
        additional scrutiny in proportion to how much of it was generated.

        As with the output of any tooling, the result may be incorrect or
        inappropriate. You are expected to understand and to be able to
        defend everything you submit. If you are unable to do so, then do
        not submit the resulting changes.

        If you do so anyway, maintainers are entitled to reject your series
        without detailed review.

In general, if you are a newcomer to mm, we expect you to do smaller work
before moving on to larger changes, so you build understanding of both the
technical aspects of mm and how we do things.

--
Cheers, Lorenzo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
  2026-09-29  5:56 [PATCH] mm/mlock: fix zero-length request normalization and integer overflows Park Tae-sun
  2026-09-29  8:30 ` Jose A. Perez de Azpillaga
  2026-09-29  8:38 ` Lorenzo Stoakes (ARM)
@ 2026-09-29  8:40 ` David Hildenbrand (Arm)
  2026-09-29  8:47   ` Lorenzo Stoakes (ARM)
  2 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand (Arm) @ 2026-09-29  8:40 UTC (permalink / raw)
  To: Park Tae-sun, Andrew Morton, Liam R. Howlett, Lorenzo Stoakes
  Cc: Vlastimil Babka, Jann Horn, Pedro Falcato, Michal Hocko,
	Mike Rapoport, Suren Baghdasaryan, linux-mm, linux-kernel

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@dgu.ac.kr>
> ---
>  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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
  2026-09-29  8:40 ` David Hildenbrand (Arm)
@ 2026-09-29  8:47   ` Lorenzo Stoakes (ARM)
  2026-09-29  9:39     ` 박태선/컴퓨터·AI학부
  0 siblings, 1 reply; 6+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-29  8:47 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Park Tae-sun, Andrew Morton, Liam R. Howlett, Vlastimil Babka,
	Jann Horn, Pedro Falcato, Michal Hocko, Mike Rapoport,
	Suren Baghdasaryan, linux-mm, linux-kernel

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@dgu.ac.kr>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
  2026-09-29  8:47   ` Lorenzo Stoakes (ARM)
@ 2026-09-29  9:39     ` 박태선/컴퓨터·AI학부
  0 siblings, 0 replies; 6+ messages in thread
From: 박태선/컴퓨터·AI학부 @ 2026-09-29  9:39 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: David Hildenbrand (Arm),
	Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
	Pedro Falcato, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
	linux-mm, linux-kernel

Understood, and thank you for the feedback.

I sincerely apologize for the poor code quality and flawed helper
design. I will be much more careful with commenting and ensure future
contributions strictly adhere to kernel guidelines.

I will drop this patch.

For David's question regarding reproducers and user-visible issues, in
case it is helpful if the core team addresses this later,
here is what is observed from a small test program wrote checking
/proc/self/smaps on an unpatched kernel:

$ ./test_mlock
mlock(p, -1UL)    : ret=0, locked=0  (silent success without actually locking)
mlock(p + 5, 0)   : ret=0, locked=1  (0-length request falsely locks 4KB page)
munlock(p + 5, 0) : ret=0, locked=0  (0-length request falsely unlocks 4KB page)

Thank you all for your time and the review.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-29  9:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29  5:56 [PATCH] mm/mlock: fix zero-length request normalization and integer overflows Park Tae-sun
2026-09-29  8:30 ` Jose A. Perez de Azpillaga
2026-09-29  8:38 ` Lorenzo Stoakes (ARM)
2026-09-29  8:40 ` David Hildenbrand (Arm)
2026-09-29  8:47   ` Lorenzo Stoakes (ARM)
2026-09-29  9:39     ` 박태선/컴퓨터·AI학부

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®