[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com>
Date: Fri, 6 Dec 2024 19:42:51 +0100
From: Jann Horn <jannh@...gle.com>
To: Brian Geffon <bgeffon@...gle.com>, "Liam R. Howlett" <Liam.Howlett@...cle.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Marco Vanotti <mvanotti@...gle.com>
Subject: Re: [PATCH 0/2] mremap: Fix newaddr hint with MREMAP_DONTUNMAP
+mmap maintainers (maybe mm/mremap.c should be added to the file
pattern for "MEMORY MAPPING" in "MAINTAINERS"? I'm not sure)
On Fri, Dec 6, 2024 at 4:20 PM Brian Geffon <bgeffon@...gle.com> wrote:
> mmap(2) allows for a destination address to be specified without
> MAP_FIXED and in this situation it's a hint to get_unmapped_area().
> This address need not be page aligned because get_unmapped_area() will
> align the hint.
>
> In the case of mremap(2) with MREMAP_DONTUNMAP it shares a code path
> with MREMAP_FIXED in mremap_to(), which means this function can be
> called in 3 different scenarios: MREMAP_FIXED only, MREMAP_DONTUNMAP
> only, or MREMAP_FIXED | MREMAP_DONTUNMAP. In the second case when only
> MREMAP_DONTUNMAP is specified we don't need to do alignment or size
> checks on newaddr because they will be passed to get_unmapped_area() and
> dealt with appropriately.
>
> This patch corrects that behavior to match what non-MREMAP_DONTUNMAP
> mremap(2) and mmap(2) do. This odd behavioral difference was reported by
> Marco Vanotti. Additionally, I've included a self test to validate this
> behavior.
Marco pointed me to this; I had no idea mremap() had this undocumented
behavior where it takes a hint address. The mremap() manpage is
currently wrong about this, it sort of implies that the new_address
argument is only used if MREMAP_FIXED is set.
Marco also noticed that upstream glibc now assumes this behavior:
https://sourceware.org/git/?p=glibc.git;a=commit;h=6c40cb0e9f893d49dc7caee580a055de53562206
Debian also has a test that explicitly checks for this behavior:
https://sources.debian.org/src/glibc/2.40-4/debian/patches/git-updates.diff/?hl=22820#L22818
I guess it's too late to remove that behavior at this point, and the
right thing to do is to update the manpage?
Powered by blists - more mailing lists