[RFC PATCH 0/6] arm64: untag user pointers passed to the kernel

arm64 has a feature called Top Byte Ignore, which allows to embed pointer
tags into the top byte of each pointer. Userspace programs (such as
HWASan, a memory debugging tool [1]) might use this feature and pass
tagged user pointers to the kernel through syscalls or other interfaces.

This patch makes a few of the kernel interfaces accept tagged user
pointers. The kernel is already able to handle user faults with tagged
pointers and has the untagged_addr macro, which this patchset reuses.

We're not trying to cover all possible ways the kernel accepts user
pointers in one patchset, so this one should be considered as a start.
It would be nice to learn about the interfaces that I missed though.

Sending this as an RFC, as I'm not sure if this should be committed as is,
and would like to receive some feedback.

Thanks!

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Andrey Konovalov (6):
  arm64: add type casts to untagged_addr macro
  arm64: untag user addresses in copy_from_user and others
  mm, arm64: untag user addresses in memory syscalls
  mm, arm64: untag user addresses in mm/gup.c
  lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
  arch: add untagged_addr definition for other arches

 arch/alpha/include/asm/uaccess.h      |  2 ++
 arch/arc/include/asm/uaccess.h        |  1 +
 arch/arm/include/asm/uaccess.h        |  2 ++
 arch/arm64/include/asm/uaccess.h      |  9 +++++++--
 arch/blackfin/include/asm/uaccess.h   |  2 ++
 arch/c6x/include/asm/uaccess.h        |  2 ++
 arch/cris/include/asm/uaccess.h       |  2 ++
 arch/frv/include/asm/uaccess.h        |  2 ++
 arch/ia64/include/asm/uaccess.h       |  2 ++
 arch/m32r/include/asm/uaccess.h       |  2 ++
 arch/m68k/include/asm/uaccess.h       |  2 ++
 arch/metag/include/asm/uaccess.h      |  2 ++
 arch/microblaze/include/asm/uaccess.h |  2 ++
 arch/mips/include/asm/uaccess.h       |  2 ++
 arch/mn10300/include/asm/uaccess.h    |  2 ++
 arch/nios2/include/asm/uaccess.h      |  2 ++
 arch/openrisc/include/asm/uaccess.h   |  2 ++
 arch/parisc/include/asm/uaccess.h     |  2 ++
 arch/powerpc/include/asm/uaccess.h    |  2 ++
 arch/riscv/include/asm/uaccess.h      |  2 ++
 arch/score/include/asm/uaccess.h      |  2 ++
 arch/sh/include/asm/uaccess.h         |  2 ++
 arch/sparc/include/asm/uaccess.h      |  2 ++
 arch/tile/include/asm/uaccess.h       |  2 ++
 arch/x86/include/asm/uaccess.h        |  2 ++
 arch/xtensa/include/asm/uaccess.h     |  2 ++
 include/asm-generic/uaccess.h         |  2 ++
 lib/strncpy_from_user.c               |  2 ++
 lib/strnlen_user.c                    |  2 ++
 mm/gup.c                              | 12 ++++++++++++
 mm/madvise.c                          |  2 ++
 mm/mempolicy.c                        |  6 ++++++
 mm/mincore.c                          |  2 ++
 mm/mlock.c                            |  5 +++++
 mm/mmap.c                             |  9 +++++++++
 mm/mprotect.c                         |  2 ++
 mm/mremap.c                           |  2 ++
 mm/msync.c                            |  3 +++
 38 files changed, 105 insertions(+), 2 deletions(-)

-- 
2.16.2.395.g2e18187dfd-goog

This patch makes the untagged_addr macro accept all kinds of address types
(void *, unsigned long, etc.) and allows not to specify type casts in each
place where it is used. This is done by using __typeof__.

Signed-off-by: Andrey Konovalov <redacted>
---
 arch/arm64/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index e66b0fca99c2..2d6451cbaa86 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h

@@ -102,7 +102,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
  * up with a tagged userland pointer. Clear the tag to get a sane pointer to
  * pass on to access_ok(), for instance.
  */
-#define untagged_addr(addr)		sign_extend64(addr, 55)
+#define untagged_addr(addr)		\
+	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
 #define access_ok(type, addr, size)	__range_ok(addr, size)
 #define user_addr_max			get_fs

-- 
2.16.2.395.g2e18187dfd-goog

copy_from_user (and a few other similar functions) are used to copy data
from user memory into the kernel memory or vice versa. Since a user can
provided a tagged pointer to one of the syscalls that use copy_from_user,
we need to correctly handle such pointers.

Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr.

Signed-off-by: Andrey Konovalov <redacted>
---
 arch/arm64/include/asm/uaccess.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 2d6451cbaa86..24a221678fe3 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h

@@ -105,7 +105,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
 #define untagged_addr(addr)		\
 	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
-#define access_ok(type, addr, size)	__range_ok(addr, size)
+#define access_ok(type, addr, size)	\
+	__range_ok(untagged_addr(addr), size)
 #define user_addr_max			get_fs
 
 #define _ASM_EXTABLE(from, to)						\

@@ -238,12 +239,15 @@ static inline void uaccess_enable_not_uao(void)
 /*
  * Sanitise a uaccess pointer such that it becomes NULL if above the
  * current addr_limit.
+ * Also untag user pointers that have the top byte tag set.
  */
 #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr)
 static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
 {
 	void __user *safe_ptr;
 
+	ptr = untagged_addr(ptr);
+
 	asm volatile(
 	"	bics	xzr, %1, %2\n"
 	"	csel	%0, %1, xzr, eq\n"

-- 
2.16.2.395.g2e18187dfd-goog

Memory subsystem syscalls accept user addresses as arguments, but don't use
copy_from_user and other similar functions, so we need to handle this case
separately.

Untag user pointers passed to madvise, mbind, get_mempolicy, mincore,
mlock, mlock2, brk, mmap_pgoff, old_mmap, munmap, remap_file_pages,
mprotect, pkey_mprotect, mremap and msync.

Signed-off-by: Andrey Konovalov <redacted>
---
 mm/madvise.c   | 2 ++
 mm/mempolicy.c | 6 ++++++
 mm/mincore.c   | 2 ++
 mm/mlock.c     | 5 +++++
 mm/mmap.c      | 9 +++++++++
 mm/mprotect.c  | 2 ++
 mm/mremap.c    | 2 ++
 mm/msync.c     | 3 +++
 8 files changed, 31 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index 4d3c922ea1a1..909d6ba09031 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c

@@ -798,6 +798,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 	size_t len;
 	struct blk_plug plug;
 
+	start = untagged_addr(start);
+
 	if (!madvise_behavior_valid(behavior))
 		return error;
 

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d879f1d8a44a..79d33a570c60 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c

@@ -1344,6 +1344,8 @@ SYSCALL_DEFINE6(mbind, unsigned long, start, unsigned long, len,
 	int err;
 	unsigned short mode_flags;
 
+	start = untagged_addr(start);
+
 	mode_flags = mode & MPOL_MODE_FLAGS;
 	mode &= ~MPOL_MODE_FLAGS;
 	if (mode >= MPOL_MAX)

@@ -1479,6 +1481,8 @@ SYSCALL_DEFINE5(get_mempolicy, int __user *, policy,
 	int uninitialized_var(pval);
 	nodemask_t nodes;
 
+	addr = untagged_addr(addr);
+
 	if (nmask != NULL && maxnode < MAX_NUMNODES)
 		return -EINVAL;
 

@@ -1557,6 +1561,8 @@ COMPAT_SYSCALL_DEFINE6(mbind, compat_ulong_t, start, compat_ulong_t, len,
 	unsigned long nr_bits, alloc_size;
 	nodemask_t bm;
 
+	start = untagged_addr(start);
+
 	nr_bits = min_t(unsigned long, maxnode-1, MAX_NUMNODES);
 	alloc_size = ALIGN(nr_bits, BITS_PER_LONG) / 8;
 

diff --git a/mm/mincore.c b/mm/mincore.c
index fc37afe226e6..b59cf8fa3050 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c

@@ -228,6 +228,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, len,
 	unsigned long pages;
 	unsigned char *tmp;
 
+	start = untagged_addr(start);
+
 	/* Check the start address: needs to be page-aligned.. */
 	if (start & ~PAGE_MASK)
 		return -EINVAL;

diff --git a/mm/mlock.c b/mm/mlock.c
index 74e5a6547c3d..2f456a458cac 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c

@@ -714,6 +714,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
 
 SYSCALL_DEFINE2(mlock, unsigned long, start, size_t, len)
 {
+	start = untagged_addr(start);
 	return do_mlock(start, len, VM_LOCKED);
 }
 

@@ -721,6 +722,8 @@ SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
 {
 	vm_flags_t vm_flags = VM_LOCKED;
 
+	start = untagged_addr(start);
+
 	if (flags & ~MLOCK_ONFAULT)
 		return -EINVAL;
 

@@ -734,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
 {
 	int ret;
 
+	start = untagged_addr(start);
+
 	len = PAGE_ALIGN(len + (offset_in_page(start)));
 	start &= PAGE_MASK;
 

diff --git a/mm/mmap.c b/mm/mmap.c
index 9efdc021ad22..b63362c45cde 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c

@@ -189,6 +189,8 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
 	bool populate;
 	LIST_HEAD(uf);
 
+	brk = untagged_addr(brk);
+
 	if (down_write_killable(&mm->mmap_sem))
 		return -EINTR;
 

@@ -1495,6 +1497,8 @@ SYSCALL_DEFINE6(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);

@@ -1556,6 +1560,8 @@ SYSCALL_DEFINE1(old_mmap, struct mmap_arg_struct __user *, arg)
 	if (offset_in_page(a.offset))
 		return -EINVAL;
 
+	a.addr = untagged_addr(a.addr);
+
 	return sys_mmap_pgoff(a.addr, a.len, a.prot, a.flags, a.fd,
 			      a.offset >> PAGE_SHIFT);
 }

@@ -2751,6 +2757,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);
 }

@@ -2769,6 +2776,8 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 	unsigned long ret = -EINVAL;
 	struct file *file;
 
+	start = untagged_addr(start);
+
 	pr_warn_once("%s (%d) uses deprecated remap_file_pages() syscall. See Documentation/vm/remap_file_pages.txt.\n",
 		     current->comm, current->pid);
 

diff --git a/mm/mprotect.c b/mm/mprotect.c
index e3309fcf586b..73d2a6befcf9 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c

@@ -519,6 +519,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
 SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot)
 {
+	start = untagged_addr(start);
 	return do_mprotect_pkey(start, len, prot, -1);
 }
 

@@ -527,6 +528,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot, int, pkey)
 {
+	start = untagged_addr(start);
 	return do_mprotect_pkey(start, len, prot, pkey);
 }
 

diff --git a/mm/mremap.c b/mm/mremap.c
index 049470aa1e3e..e42863a135de 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c

@@ -533,6 +533,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 	LIST_HEAD(uf_unmap_early);
 	LIST_HEAD(uf_unmap);
 
+	addr = untagged_addr(addr);
+
 	if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
 		return ret;
 

diff --git a/mm/msync.c b/mm/msync.c
index ef30a429623a..03a977558f9f 100644
--- a/mm/msync.c
+++ b/mm/msync.c

@@ -37,12 +37,15 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 	int unmapped_error = 0;
 	int error = -EINVAL;
 
+	start = untagged_addr(start);
+
 	if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC))
 		goto out;
 	if (offset_in_page(start))
 		goto out;
 	if ((flags & MS_ASYNC) && (flags & MS_SYNC))
 		goto out;
+
 	error = -ENOMEM;
 	len = (len + ~PAGE_MASK) & PAGE_MASK;
 	end = start + len;

-- 
2.16.2.395.g2e18187dfd-goog

mm/gup.c provides a kernel interface that accepts user addresses and
manipulates user pages directly (for example get_user_pages, that is used
by the futex syscall). Here we also need to handle the case of tagged user
pointers.

Untag addresses passed to this interface.

Signed-off-by: Andrey Konovalov <redacted>
---
 mm/gup.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index 1b46e6e74881..4d820c4792d7 100644
--- a/mm/gup.c
+++ b/mm/gup.c

@@ -386,6 +386,8 @@ struct page *follow_page_mask(struct vm_area_struct *vma,
 	struct page *page;
 	struct mm_struct *mm = vma->vm_mm;
 
+	address = untagged_addr(address);
+
 	*page_mask = 0;
 
 	/* make this handle hugepd */

@@ -647,6 +649,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
 	if (!nr_pages)
 		return 0;
 
+	start = untagged_addr(start);
+
 	VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
 
 	/*

@@ -801,6 +805,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
 	struct vm_area_struct *vma;
 	int ret, major = 0;
 
+	address = untagged_addr(address);
+
 	if (unlocked)
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY;
 

@@ -854,6 +860,8 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
 	long ret, pages_done;
 	bool lock_dropped;
 
+	start = untagged_addr(start);
+
 	if (locked) {
 		/* if VM_FAULT_RETRY can be returned, vmas become invalid */
 		BUG_ON(vmas);

@@ -1746,6 +1754,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
 	unsigned long flags;
 	int nr = 0;
 
+	start = untagged_addr(start);
+
 	start &= PAGE_MASK;
 	addr = start;
 	len = (unsigned long) nr_pages << PAGE_SHIFT;

@@ -1798,6 +1808,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 	unsigned long addr, len, end;
 	int nr = 0, ret = 0;
 
+	start = untagged_addr(start);
+
 	start &= PAGE_MASK;
 	addr = start;
 	len = (unsigned long) nr_pages << PAGE_SHIFT;

-- 
2.16.2.395.g2e18187dfd-goog

strncpy_from_user and strnlen_user accept user addresses as arguments, and
do not go through the same path as copy_from_user and others, so here we
need to separately handle the case of tagged user addresses as well.

Untag user pointers passed to these functions.

Signed-off-by: Andrey Konovalov <redacted>
---
 lib/strncpy_from_user.c | 2 ++
 lib/strnlen_user.c      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index b53e1b5d80f4..97467cd2bc59 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c

@@ -106,6 +106,8 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	src = untagged_addr(src);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)src;
 	if (likely(src_addr < max_addr)) {

diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 60d0bbda8f5e..8b5f56466e00 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c

@@ -108,6 +108,8 @@ long strnlen_user(const char __user *str, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	str = untagged_addr(str);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)str;
 	if (likely(src_addr < max_addr)) {

-- 
2.16.2.395.g2e18187dfd-goog

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Define it as a noop for all other architectures besides arm64.

Signed-off-by: Andrey Konovalov <redacted>
---
 arch/alpha/include/asm/uaccess.h      | 2 ++
 arch/arc/include/asm/uaccess.h        | 1 +
 arch/arm/include/asm/uaccess.h        | 2 ++
 arch/blackfin/include/asm/uaccess.h   | 2 ++
 arch/c6x/include/asm/uaccess.h        | 2 ++
 arch/cris/include/asm/uaccess.h       | 2 ++
 arch/frv/include/asm/uaccess.h        | 2 ++
 arch/ia64/include/asm/uaccess.h       | 2 ++
 arch/m32r/include/asm/uaccess.h       | 2 ++
 arch/m68k/include/asm/uaccess.h       | 2 ++
 arch/metag/include/asm/uaccess.h      | 2 ++
 arch/microblaze/include/asm/uaccess.h | 2 ++
 arch/mips/include/asm/uaccess.h       | 2 ++
 arch/mn10300/include/asm/uaccess.h    | 2 ++
 arch/nios2/include/asm/uaccess.h      | 2 ++
 arch/openrisc/include/asm/uaccess.h   | 2 ++
 arch/parisc/include/asm/uaccess.h     | 2 ++
 arch/powerpc/include/asm/uaccess.h    | 2 ++
 arch/riscv/include/asm/uaccess.h      | 2 ++
 arch/score/include/asm/uaccess.h      | 2 ++
 arch/sh/include/asm/uaccess.h         | 2 ++
 arch/sparc/include/asm/uaccess.h      | 2 ++
 arch/tile/include/asm/uaccess.h       | 2 ++
 arch/x86/include/asm/uaccess.h        | 2 ++
 arch/xtensa/include/asm/uaccess.h     | 2 ++
 include/asm-generic/uaccess.h         | 2 ++
 26 files changed, 51 insertions(+)

diff --git a/arch/alpha/include/asm/uaccess.h b/arch/alpha/include/asm/uaccess.h
index 87d8c4f0307d..09d136bb4ff5 100644
--- a/arch/alpha/include/asm/uaccess.h
+++ b/arch/alpha/include/asm/uaccess.h

@@ -2,6 +2,8 @@
 #ifndef __ALPHA_UACCESS_H
 #define __ALPHA_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/arc/include/asm/uaccess.h b/arch/arc/include/asm/uaccess.h
index c9173c02081c..2a04b7a4aada 100644
--- a/arch/arc/include/asm/uaccess.h
+++ b/arch/arc/include/asm/uaccess.h

@@ -26,6 +26,7 @@
 
 #include <linux/string.h>	/* for generic string functions */
 
+#define untagged_addr(addr)	addr
 
 #define __kernel_ok		(uaccess_kernel())
 

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 0bf2347495f1..7d4f4e4021f2 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h

@@ -19,6 +19,8 @@
 
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * These two functions allow hooking accesses to userspace to increase
  * system integrity by ensuring that the kernel can not inadvertantly

diff --git a/arch/blackfin/include/asm/uaccess.h b/arch/blackfin/include/asm/uaccess.h
index 45da4bcb050e..fb6bdc54e7bd 100644
--- a/arch/blackfin/include/asm/uaccess.h
+++ b/arch/blackfin/include/asm/uaccess.h

@@ -18,6 +18,8 @@
 #include <asm/segment.h>
 #include <asm/sections.h>
 
+#define untagged_addr(addr)	addr
+
 #define get_ds()        (KERNEL_DS)
 #define get_fs()        (current_thread_info()->addr_limit)
 

diff --git a/arch/c6x/include/asm/uaccess.h b/arch/c6x/include/asm/uaccess.h
index ba6756879f00..f187696cf440 100644
--- a/arch/c6x/include/asm/uaccess.h
+++ b/arch/c6x/include/asm/uaccess.h

@@ -9,6 +9,8 @@
 #ifndef _ASM_C6X_UACCESS_H
 #define _ASM_C6X_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <linux/string.h>

diff --git a/arch/cris/include/asm/uaccess.h b/arch/cris/include/asm/uaccess.h
index 3b42ab0cae93..86d8fbd200c4 100644
--- a/arch/cris/include/asm/uaccess.h
+++ b/arch/cris/include/asm/uaccess.h

@@ -19,6 +19,8 @@
 #include <asm/processor.h>
 #include <asm/page.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/frv/include/asm/uaccess.h b/arch/frv/include/asm/uaccess.h
index ff9562dc6825..be21b42bde09 100644
--- a/arch/frv/include/asm/uaccess.h
+++ b/arch/frv/include/asm/uaccess.h

@@ -12,6 +12,8 @@
 #ifndef _ASM_UACCESS_H
 #define _ASM_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 /*
  * User space memory access functions
  */

diff --git a/arch/ia64/include/asm/uaccess.h b/arch/ia64/include/asm/uaccess.h
index a74524f2d625..1c46bf1c4f73 100644
--- a/arch/ia64/include/asm/uaccess.h
+++ b/arch/ia64/include/asm/uaccess.h

@@ -42,6 +42,8 @@
 #include <asm/io.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * For historical reasons, the following macros are grossly misnamed:
  */

diff --git a/arch/m32r/include/asm/uaccess.h b/arch/m32r/include/asm/uaccess.h
index 9d89bc3d8181..6e0fe6b215be 100644
--- a/arch/m32r/include/asm/uaccess.h
+++ b/arch/m32r/include/asm/uaccess.h

@@ -16,6 +16,8 @@
 #include <asm/setup.h>
 #include <linux/prefetch.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h
index e896466a41a4..02e0c5878ad5 100644
--- a/arch/m68k/include/asm/uaccess.h
+++ b/arch/m68k/include/asm/uaccess.h

@@ -5,3 +5,5 @@
 #include <asm/uaccess_mm.h>
 #endif
 #include <asm/extable.h>
+
+#define untagged_addr(addr)	addr

diff --git a/arch/metag/include/asm/uaccess.h b/arch/metag/include/asm/uaccess.h
index a5311eb36e32..1b2f0478868a 100644
--- a/arch/metag/include/asm/uaccess.h
+++ b/arch/metag/include/asm/uaccess.h

@@ -14,6 +14,8 @@
  * For historical reasons, these macros are grossly misnamed.
  */
 
+#define untagged_addr(addr)	addr
+
 #define MAKE_MM_SEG(s)  ((mm_segment_t) { (s) })
 
 #define KERNEL_DS       MAKE_MM_SEG(0xFFFFFFFF)

diff --git a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
index 81f16aadbf9e..a66bc26660c3 100644
--- a/arch/microblaze/include/asm/uaccess.h
+++ b/arch/microblaze/include/asm/uaccess.h

@@ -20,6 +20,8 @@
 #include <asm/extable.h>
 #include <linux/string.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * On Microblaze the fs value is actually the top of the corresponding
  * address space.

diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h
index b71306947290..2db7606c388b 100644
--- a/arch/mips/include/asm/uaccess.h
+++ b/arch/mips/include/asm/uaccess.h

@@ -16,6 +16,8 @@
 #include <asm/asm-eva.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/mn10300/include/asm/uaccess.h b/arch/mn10300/include/asm/uaccess.h
index 5af468fd1359..6604699b34b6 100644
--- a/arch/mn10300/include/asm/uaccess.h
+++ b/arch/mn10300/include/asm/uaccess.h

@@ -17,6 +17,8 @@
 #include <linux/kernel.h>
 #include <asm/page.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/nios2/include/asm/uaccess.h b/arch/nios2/include/asm/uaccess.h
index dfa3c7cb30b4..36152a7302a8 100644
--- a/arch/nios2/include/asm/uaccess.h
+++ b/arch/nios2/include/asm/uaccess.h

@@ -19,6 +19,8 @@
 
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * Segment stuff
  */

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index bbf5c79cce7a..5b43d13ab363 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h

@@ -27,6 +27,8 @@
 #include <asm/page.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index ea70e36ce6af..b0f3cd529c8d 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h

@@ -11,6 +11,8 @@
 #include <linux/bug.h>
 #include <linux/string.h>
 
+#define untagged_addr(addr)	addr
+
 #define KERNEL_DS	((mm_segment_t){0})
 #define USER_DS 	((mm_segment_t){1})
 

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..07ae1c318166 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h

@@ -8,6 +8,8 @@
 #include <asm/page.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 14b0b22fb578..e774239aac24 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h

@@ -25,6 +25,8 @@
 #include <asm/byteorder.h>
 #include <asm/asm.h>
 
+#define untagged_addr(addr)	addr
+
 #define __enable_user_access()							\
 	__asm__ __volatile__ ("csrs sstatus, %0" : : "r" (SR_SUM) : "memory")
 #define __disable_user_access()							\

diff --git a/arch/score/include/asm/uaccess.h b/arch/score/include/asm/uaccess.h
index a233f3236846..fd16c2a71091 100644
--- a/arch/score/include/asm/uaccess.h
+++ b/arch/score/include/asm/uaccess.h

@@ -5,6 +5,8 @@
 #include <linux/kernel.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 #define get_ds()		(KERNEL_DS)
 #define get_fs()		(current_thread_info()->addr_limit)
 #define segment_eq(a, b)	((a).seg == (b).seg)

diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
index 32eb56e00c11..31f3ea075190 100644
--- a/arch/sh/include/asm/uaccess.h
+++ b/arch/sh/include/asm/uaccess.h

@@ -5,6 +5,8 @@
 #include <asm/segment.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 #define __addr_ok(addr) \
 	((unsigned long __force)(addr) < current_thread_info()->addr_limit.seg)
 

diff --git a/arch/sparc/include/asm/uaccess.h b/arch/sparc/include/asm/uaccess.h
index dd85bc2c2cad..70c2f5ea09ce 100644
--- a/arch/sparc/include/asm/uaccess.h
+++ b/arch/sparc/include/asm/uaccess.h

@@ -7,6 +7,8 @@
 #include <asm/uaccess_32.h>
 #endif
 
+#define untagged_addr(addr)	addr
+
 #define user_addr_max() \
 	(uaccess_kernel() ? ~0UL : TASK_SIZE)
 

diff --git a/arch/tile/include/asm/uaccess.h b/arch/tile/include/asm/uaccess.h
index cb4fbe7e4f88..7d365b087dcb 100644
--- a/arch/tile/include/asm/uaccess.h
+++ b/arch/tile/include/asm/uaccess.h

@@ -22,6 +22,8 @@
 #include <asm/processor.h>
 #include <asm/page.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index aae77eb8491c..3c233fbdd32b 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h

@@ -12,6 +12,8 @@
 #include <asm/smap.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with

diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index f1158b4c629c..130e419c4d6e 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h

@@ -20,6 +20,8 @@
 #include <asm/types.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should
  * be performed or not.  If get_fs() == USER_DS, checking is

diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h
index 6b2e63df2739..2c46d2253dba 100644
--- a/include/asm-generic/uaccess.h
+++ b/include/asm-generic/uaccess.h

@@ -35,6 +35,8 @@ static inline void set_fs(mm_segment_t fs)
 #define segment_eq(a, b) ((a).seg == (b).seg)
 #endif
 
+#define untagged_addr(addr) addr
+
 #define access_ok(type, addr, size) __access_ok((unsigned long)(addr),(size))
 
 /*

-- 
2.16.2.395.g2e18187dfd-goog

On Fri, Mar 9, 2018 at 3:02 PM, Andrey Konovalov [off-list ref] wrote:

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Define it as a noop for all other architectures besides arm64.

Signed-off-by: Andrey Konovalov <redacted>
---
 arch/alpha/include/asm/uaccess.h      | 2 ++
 arch/arc/include/asm/uaccess.h        | 1 +
 arch/arm/include/asm/uaccess.h        | 2 ++
 arch/blackfin/include/asm/uaccess.h   | 2 ++
 arch/c6x/include/asm/uaccess.h        | 2 ++
 arch/cris/include/asm/uaccess.h       | 2 ++
 arch/frv/include/asm/uaccess.h        | 2 ++
 arch/ia64/include/asm/uaccess.h       | 2 ++
 arch/m32r/include/asm/uaccess.h       | 2 ++
 arch/m68k/include/asm/uaccess.h       | 2 ++
 arch/metag/include/asm/uaccess.h      | 2 ++
 arch/microblaze/include/asm/uaccess.h | 2 ++
 arch/mips/include/asm/uaccess.h       | 2 ++
 arch/mn10300/include/asm/uaccess.h    | 2 ++
 arch/nios2/include/asm/uaccess.h      | 2 ++
 arch/openrisc/include/asm/uaccess.h   | 2 ++
 arch/parisc/include/asm/uaccess.h     | 2 ++
 arch/powerpc/include/asm/uaccess.h    | 2 ++
 arch/riscv/include/asm/uaccess.h      | 2 ++
 arch/score/include/asm/uaccess.h      | 2 ++
 arch/sh/include/asm/uaccess.h         | 2 ++
 arch/sparc/include/asm/uaccess.h      | 2 ++
 arch/tile/include/asm/uaccess.h       | 2 ++
 arch/x86/include/asm/uaccess.h        | 2 ++
 arch/xtensa/include/asm/uaccess.h     | 2 ++
 include/asm-generic/uaccess.h         | 2 ++
 26 files changed, 51 insertions(+)

I have patches to remove the blackfin, cris, frv, m32r, metag, mn10300,
score, tile and unicore32 architectures from the kernel, these should be
part of linux-next in the next few days. It's not a big issue, but if you keep
patching them, this will cause a merge conflict.

It might be easier to drop them from your patch as well.

    Arnd

Hi Andrey,

On 09/03/18 14:01, Andrey Konovalov wrote:

arm64 has a feature called Top Byte Ignore, which allows to embed pointer
tags into the top byte of each pointer. Userspace programs (such as
HWASan, a memory debugging tool [1]) might use this feature and pass
tagged user pointers to the kernel through syscalls or other interfaces.

If you propose changing the ABI, then 
Documentation/arm64/tagged-pointers.txt needs to reflect the new one, 
since passing nonzero tags via syscalls is currently explicitly forbidden.

Robin.

This patch makes a few of the kernel interfaces accept tagged user
pointers. The kernel is already able to handle user faults with tagged
pointers and has the untagged_addr macro, which this patchset reuses.

We're not trying to cover all possible ways the kernel accepts user
pointers in one patchset, so this one should be considered as a start.
It would be nice to learn about the interfaces that I missed though.

Sending this as an RFC, as I'm not sure if this should be committed as is,
and would like to receive some feedback.

Thanks!

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Andrey Konovalov (6):
   arm64: add type casts to untagged_addr macro
   arm64: untag user addresses in copy_from_user and others
   mm, arm64: untag user addresses in memory syscalls
   mm, arm64: untag user addresses in mm/gup.c
   lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
   arch: add untagged_addr definition for other arches

  arch/alpha/include/asm/uaccess.h      |  2 ++
  arch/arc/include/asm/uaccess.h        |  1 +
  arch/arm/include/asm/uaccess.h        |  2 ++
  arch/arm64/include/asm/uaccess.h      |  9 +++++++--
  arch/blackfin/include/asm/uaccess.h   |  2 ++
  arch/c6x/include/asm/uaccess.h        |  2 ++
  arch/cris/include/asm/uaccess.h       |  2 ++
  arch/frv/include/asm/uaccess.h        |  2 ++
  arch/ia64/include/asm/uaccess.h       |  2 ++
  arch/m32r/include/asm/uaccess.h       |  2 ++
  arch/m68k/include/asm/uaccess.h       |  2 ++
  arch/metag/include/asm/uaccess.h      |  2 ++
  arch/microblaze/include/asm/uaccess.h |  2 ++
  arch/mips/include/asm/uaccess.h       |  2 ++
  arch/mn10300/include/asm/uaccess.h    |  2 ++
  arch/nios2/include/asm/uaccess.h      |  2 ++
  arch/openrisc/include/asm/uaccess.h   |  2 ++
  arch/parisc/include/asm/uaccess.h     |  2 ++
  arch/powerpc/include/asm/uaccess.h    |  2 ++
  arch/riscv/include/asm/uaccess.h      |  2 ++
  arch/score/include/asm/uaccess.h      |  2 ++
  arch/sh/include/asm/uaccess.h         |  2 ++
  arch/sparc/include/asm/uaccess.h      |  2 ++
  arch/tile/include/asm/uaccess.h       |  2 ++
  arch/x86/include/asm/uaccess.h        |  2 ++
  arch/xtensa/include/asm/uaccess.h     |  2 ++
  include/asm-generic/uaccess.h         |  2 ++
  lib/strncpy_from_user.c               |  2 ++
  lib/strnlen_user.c                    |  2 ++
  mm/gup.c                              | 12 ++++++++++++
  mm/madvise.c                          |  2 ++
  mm/mempolicy.c                        |  6 ++++++
  mm/mincore.c                          |  2 ++
  mm/mlock.c                            |  5 +++++
  mm/mmap.c                             |  9 +++++++++
  mm/mprotect.c                         |  2 ++
  mm/mremap.c                           |  2 ++
  mm/msync.c                            |  3 +++
  38 files changed, 105 insertions(+), 2 deletions(-)

On 09/03/18 14:02, Andrey Konovalov wrote:

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Would it not suffice to have an "#ifndef untagged_addr..." fallback in 
linux/uaccess.h?

Robin.

On Fri, Mar 9, 2018 at 3:16 PM, Robin Murphy [off-list ref] wrote:

On 09/03/18 14:02, Andrey Konovalov wrote:

quoted

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Would it not suffice to have an "#ifndef untagged_addr..." fallback in
linux/uaccess.h?

Hi Robin!

This approach is much better, I'll try it. This will also solve the
merge issues that Arnd mentioned.

Thanks!

On Fri, Mar 9, 2018 at 3:15 PM, Robin Murphy [off-list ref] wrote:

Hi Andrey,

On 09/03/18 14:01, Andrey Konovalov wrote:

quoted

arm64 has a feature called Top Byte Ignore, which allows to embed pointer
tags into the top byte of each pointer. Userspace programs (such as
HWASan, a memory debugging tool [1]) might use this feature and pass
tagged user pointers to the kernel through syscalls or other interfaces.

If you propose changing the ABI, then
Documentation/arm64/tagged-pointers.txt needs to reflect the new one, since
passing nonzero tags via syscalls is currently explicitly forbidden.

Robin.

Hi Robin!

I will include changes to Documentation/arm64/tagged-pointers.txt in
the next version.

Thanks!