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

[Andrey Konovalov <andreyknvl@google.com>]

On Tue, Feb 18, 2020 at 1:34 PM Will Deacon <will@kernel.org> wrote:
>
> 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.

Evgenii, do you know if this will cause any issues for HWASAN?

> >
> > 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