On Tue, Feb 18, 2020 at 1:23 PM Catalin Marinas <catalin.marinas@xxxxxxx> 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.
>
> 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@xxxxxxxxxxxxxxx> # 5.4.x-
> Cc: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Florian Weimer <fweimer@xxxxxxxxxx>
> Reported-by: Victor Stinner <vstinner@xxxxxxxxxx>
> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
Acked-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> ---
> 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.
>
> 2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
> relaxation is disabled by default and the application thread needs to
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 6756b8bb0033..d681a20eb4ea 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -195,8 +195,6 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
> bool downgraded = false;
> LIST_HEAD(uf);
>
> - brk = untagged_addr(brk);
> -
> if (down_write_killable(&mm->mmap_sem))
> return -EINTR;
>
> @@ -1557,8 +1555,6 @@ unsigned long ksys_mmap_pgoff(unsigned long addr, unsigned long len,
> struct file *file = NULL;
> unsigned long retval;
>
> - addr = untagged_addr(addr);
> -
> if (!(flags & MAP_ANONYMOUS)) {
> audit_mmap_fd(fd, flags);
> file = fget(fd);
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 122938dcec15..af363063ea23 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -607,7 +607,6 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
> LIST_HEAD(uf_unmap);
>
> addr = untagged_addr(addr);
> - new_addr = untagged_addr(new_addr);
>
> if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
> return ret;
