Re: [PATCH v8 1/5] mm: untag user pointers in mmap/munmap/mremap/brk

[Will Deacon]

On Thu, Aug 22, 2019 at 04:41:25PM -0700, Andrew Morton wrote:
> On Mon, 19 Aug 2019 17:28:51 +0100 Will Deacon  wrote:
> 
> > On Thu, Aug 15, 2019 at 04:43:59PM +0100, Catalin Marinas wrote:
> > > There isn't a good reason to differentiate between the user address
> > > space layout modification syscalls and the other memory
> > > permission/attributes ones (e.g. mprotect, madvise) w.r.t. the tagged
> > > address ABI. Untag the user addresses on entry to these functions.
> > > 
> > > Signed-off-by: Catalin Marinas 
> > > ---
> > >  mm/mmap.c   | 5 +
> > >  mm/mremap.c | 6 +-
> > >  2 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > Acked-by: Will Deacon 
> > 
> > Andrew -- please can you pick this patch up? I'll take the rest of the
> > series via arm64 once we've finished discussing the wording details.
> > 
> 
> Sure, I grabbed the patch from the v9 series.

Thanks, Andrew.

> But please feel free to include this in the arm64 tree - I'll autodrop
> my copy if this turns up in linux-next.

I'd prefer for this one to go via you so that it can sit with the rest of
the core changes relating to tagged addresses. Obviously please yell if
you run into any issues with it!

Cheers,

Will

Re: [PATCH v8 1/5] mm: untag user pointers in mmap/munmap/mremap/brk

[Andrew Morton]

On Mon, 19 Aug 2019 17:28:51 +0100 Will Deacon  wrote:

> On Thu, Aug 15, 2019 at 04:43:59PM +0100, Catalin Marinas wrote:
> > There isn't a good reason to differentiate between the user address
> > space layout modification syscalls and the other memory
> > permission/attributes ones (e.g. mprotect, madvise) w.r.t. the tagged
> > address ABI. Untag the user addresses on entry to these functions.
> > 
> > Signed-off-by: Catalin Marinas 
> > ---
> >  mm/mmap.c   | 5 +
> >  mm/mremap.c | 6 +-
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> Acked-by: Will Deacon 
> 
> Andrew -- please can you pick this patch up? I'll take the rest of the
> series via arm64 once we've finished discussing the wording details.
> 

Sure, I grabbed the patch from the v9 series.

But please feel free to include this in the arm64 tree - I'll autodrop
my copy if this turns up in linux-next.

Re: [PATCH v8 1/5] mm: untag user pointers in mmap/munmap/mremap/brk

[Will Deacon]

On Thu, Aug 15, 2019 at 04:43:59PM +0100, Catalin Marinas wrote:
> There isn't a good reason to differentiate between the user address
> space layout modification syscalls and the other memory
> permission/attributes ones (e.g. mprotect, madvise) w.r.t. the tagged
> address ABI. Untag the user addresses on entry to these functions.
> 
> Signed-off-by: Catalin Marinas 
> ---
>  mm/mmap.c   | 5 +
>  mm/mremap.c | 6 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)

Acked-by: Will Deacon 

Andrew -- please can you pick this patch up? I'll take the rest of the
series via arm64 once we've finished discussing the wording details.

Thanks,

Will

Re: [PATCH v8 1/5] mm: untag user pointers in mmap/munmap/mremap/brk

[Andrey Konovalov]

On Thu, Aug 15, 2019 at 5:44 PM Catalin Marinas  wrote:
>
> There isn't a good reason to differentiate between the user address
> space layout modification syscalls and the other memory
> permission/attributes ones (e.g. mprotect, madvise) w.r.t. the tagged
> address ABI. Untag the user addresses on entry to these functions.
>
> Signed-off-by: Catalin Marinas 

Acked-by: Andrey Konovalov 

> ---
>  mm/mmap.c   | 5 +
>  mm/mremap.c | 6 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 7e8c3e8ae75f..b766b633b7ae 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -201,6 +201,8 @@ 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;
>
> @@ -1573,6 +1575,8 @@ 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);
> @@ -2874,6 +2878,7 @@ EXPORT_SYMBOL(vm_munmap);
>
>  SYSCALL_DEFINE2(munmap, unsigned long, addr, size_t, len)
>  {
> +   addr = untagged_addr(addr);
> profile_munmap(addr);
> return __vm_munmap(addr, len, true);
>  }
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 64c9a3b8be0a..1fc8a29fbe3f 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -606,12 +606,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned 
> long, old_len,
> LIST_HEAD(uf_unmap_early);
> LIST_HEAD(uf_unmap);
>
> -   /*
> -* Architectures may interpret the tag passed to mmap as a background
> -* colour for the corresponding vma. For mremap we don't allow tagged
> -* new_addr to preserve similar behaviour to mmap.
> -*/
> addr = untagged_addr(addr);
> +   new_addr = untagged_addr(new_addr);
>
> if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
> return ret;