From: "Jose A. Perez de Azpillaga" <azpijr@gmail.com>
To: Park Tae-sun <ts930@dgu.ac.kr>
Cc: Andrew Morton <akpm@linux-foundation.org>,
"Liam R. Howlett" <liam@infradead.org>,
Lorenzo Stoakes <ljs@kernel.org>,
David Hildenbrand <david@kernel.org>,
Vlastimil Babka <vbabka@kernel.org>,
Jann Horn <jannh@google.com>, Pedro Falcato <pfalcato@suse.de>,
Michal Hocko <mhocko@suse.com>, Mike Rapoport <rppt@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/mlock: fix zero-length request normalization and integer overflows
Date: Tue, 29 Sep 2026 10:30:39 +0200 [thread overview]
Message-ID: <artu7tGV5yiwErx8@gmail.com> (raw)
In-Reply-To: <179066531783.50175.13377521828379196177@dgu.ac.kr>
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
next prev parent reply other threads:[~2026-09-29 8:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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학부
2026-09-30 11:43 ` David Hildenbrand (Arm)
2026-09-30 12:14 ` Lorenzo Stoakes (ARM)
2026-09-30 16:08 ` David Hildenbrand (Arm)
2026-09-30 16:09 ` Lorenzo Stoakes (ARM)
2026-09-30 20:42 ` Jose A. Perez de Azpillaga
2026-10-01 6:56 ` David Hildenbrand (Arm)
2026-10-09 4:05 ` kernel test robot
2026-10-09 4:05 ` [LTP] " kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=artu7tGV5yiwErx8@gmail.com \
--to=azpijr@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=david@kernel.org \
--cc=jannh@google.com \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=ljs@kernel.org \
--cc=mhocko@suse.com \
--cc=pfalcato@suse.de \
--cc=rppt@kernel.org \
--cc=surenb@google.com \
--cc=ts930@dgu.ac.kr \
--cc=vbabka@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.