Re: [PATCH] mm: Avoid creating virtual address aliases in brk()/mmap()/mremap()

[Will Deacon <will@kernel.org>]

On Tue, Feb 18, 2020 at 12:23:10PM +0000, Catalin Marinas wrote:
> Currently the arm64 kernel ignores the top address byte passed to brk(),
> mmap() and mremap(). When the user is not aware of the 56-bit address
> limit or relies on the kernel to return an error, untagging such
> pointers has the potential to create address aliases in user-space.
> Passing a tagged address to munmap(), madvise() is permitted since the
> tagged pointer is expected to be inside an existing mapping.

Might be worth mentioning that this is causing real issues for existing
userspace:

https://bugzilla.redhat.com/show_bug.cgi?id=1797052

and so should be merged as a fix.

> Remove untagging in the above functions by partially reverting commit
> ce18d171cb73 ("mm: untag user pointers in mmap/munmap/mremap/brk"). In
> addition, update the arm64 tagged-address-abi.rst document accordingly.
> 
> Fixes: ce18d171cb73 ("mm: untag user pointers in mmap/munmap/mremap/brk")
> Cc: <stable@vger.kernel.org> # 5.4.x-
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Florian Weimer <fweimer@redhat.com>
> Reported-by: Victor Stinner <vstinner@redhat.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  Documentation/arm64/tagged-address-abi.rst | 7 +++++--
>  mm/mmap.c                                  | 4 ----
>  mm/mremap.c                                | 1 -
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> index d4a85d535bf9..1771a8b5712e 100644
> --- a/Documentation/arm64/tagged-address-abi.rst
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -44,8 +44,11 @@ The AArch64 Tagged Address ABI has two stages of relaxation depending
>  how the user addresses are used by the kernel:
>  
>  1. User addresses not accessed by the kernel but used for address space
> -   management (e.g. ``mmap()``, ``mprotect()``, ``madvise()``). The use
> -   of valid tagged pointers in this context is always allowed.
> +   management (e.g. ``mprotect()``, ``madvise()``). The use of valid
> +   tagged pointers in this context is allowed with the exception of
> +   ``brk()``, ``mmap()`` and the ``new_address`` argument to
> +   ``mremap()`` as these have the potential of aliasing with existing
> +   user addresses.

Given that we're backporting this to stable kernels, perhaps it's worth
a note here along the lines of:

NOTE: This behaviour changed in v5.6 and so some earlier kernels may
incorrectly accept valid tagged pointers for these system calls.

With that:

Acked-by: Will Deacon <will@kernel.org>

Happy to take this as an arm64 fix for 5.6, unless Andrew would prefer
to route it via his tree.

Will