Re: [RFC PATCH 3/5] mm: mremap: Allow new_addr to be specified as a hint

[Brian Geffon]

On Tue, Dec 10, 2024 at 1:31 PM Brian Geffon <bgeffon@xxxxxxxxxx> wrote:
>
> When using MREMAP_MAYMOVE previously the new_addr was ignored unless
> the user specified MREMAP_FIXED. This change will allow it to be
> used as a hint in that situation similar to how mmap(2) behaves.
>
> get_unmapped_area() will handle page aligning the new address hint.
>
> Signed-off-by: Brian Geffon <bgeffon@xxxxxxxxxx>
> ---
>  mm/mremap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index fdc1b0f1b38e..1d2522fba0ef 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1205,7 +1205,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
>                 if (vma->vm_flags & VM_MAYSHARE)
>                         map_flags |= MAP_SHARED;
>
> -               new_addr = get_unmapped_area(vma->vm_file, 0, new_len,
> +               new_addr = get_unmapped_area(vma->vm_file, new_addr, new_len,

Marco raised the concern that since glibc [1] handles new_addr as a
variadic argument, existing call sites which used only MREMAP_MAYMOVE
might end up passing garbage to the glibc wrapper for new_addr. I
checked and it turns out that musl does the same [2]. So I'm not sure
how this could ever be safely implemented? Worst case scenario we're
leaking stack data as a hint to mremap, yikes.

1. https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/mremap.c
2. https://git.musl-libc.org/cgit/musl/tree/src/mman/mremap.c

>                                         vma->vm_pgoff +
>                                         ((addr - vma->vm_start) >> PAGE_SHIFT),
>                                         map_flags);
> --
> 2.47.0.338.g60cca15819-goog
>