On Tue, Feb 18, 2020 at 5:07 AM Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote: > > On Tue, Feb 18, 2020 at 1:34 PM Will Deacon <will@xxxxxxxxxx> 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? Is it possible to preserve the untagging behavior when a process has opted in TBI? I have not seen an actual issue with a tagged pointer in mmap yet (I've seen two with mprotect, but not mmap or sbrk), so we should be fine either way. > > > > > > > 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> > > > --- > > > 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@xxxxxxxxxx> > > > > Happy to take this as an arm64 fix for 5.6, unless Andrew would prefer > > to route it via his tree. > > > > Will
