Skip to contentSkip to conversation

arm64: untag user pointers passed to the kernel

RFC PATCH Superseded Andrey Konovalov 7 participants 24 messages Mar 9, 2018 – Mar 16, 2018

Subsystems (24)
Memory management Arm (32-bit) x86 PowerPC +20 more subsystems

v1 of 19 Superseded Latest is v19 →

lore
Share & tools
lore

Andrey Konovalov

author
#0 lore raw

cover 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(-)
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
author
#1 lore raw

RFC PATCH 1/6 arm64: add type casts to untagged_addr macro

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 <[email protected]>
---
 arch/arm64/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/include/asm/uaccess.h +2 -1
index e66b0fca99c2..2d6451cbaa86 100644
@@ -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
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
author
#2 lore raw

RFC PATCH 2/6 arm64: untag user addresses in copy_from_user and others

In reply to Andrey Konovalov

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 <[email protected]>
---
 arch/arm64/include/asm/uaccess.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm64/include/asm/uaccess.h +5 -1
index 2d6451cbaa86..24a221678fe3 100644
@@ -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"
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
reviewer
#3 lore raw

Re: [RFC PATCH 2/6] arm64: untag user addresses in copy_from_user and others

Quoted 4 lines · Andrey Konovalov

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.

I don't think it makes sense to do this in the low-level uaccess primitives, given we're going to have to untag pointers before common code can use them, e.g. for comparisons against TASK_SIZE or user_addr_max().

I think we'll end up with subtle bugs unless we consistently untag pointers before we get to uaccess primitives. If core code does untag pointers, then it's redundant to do so here.

Thanks,
Mark.

Quoted 41 lines

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

Signed-off-by Andrey Konovalov <[email protected]>
---
 arch/arm64/include/asm/uaccess.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm64/include/asm/uaccess.h +5 -1
index 2d6451cbaa86..24a221678fe3 100644
@@ -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"
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
maintainer
#4 lore raw

Re: [RFC PATCH 2/6] arm64: untag user addresses in copy_from_user and others

Quoted 6 lines · Mark Rutland
Quoted 4 lines · Andrey Konovalov

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.

Quoted 8 lines

I don't think it makes sense to do this in the low-level uaccess primitives, given we're going to have to untag pointers before common code can use them, e.g. for comparisons against TASK_SIZE or user_addr_max().

I think we'll end up with subtle bugs unless we consistently untag pointers before we get to uaccess primitives. If core code does untag pointers, then it's redundant to do so here.

A quick "hack" below clears the tag on syscall entry (where the argument is a __user pointer). However, we still have cases in core code where the pointer is read from a structure or even passed as an unsigned long as part of a command + argument (like in ptrace).

The "hack":

---------------------------------8<-------------------------- From 6df503651f73c923d91eb695e56f977ddcc52d43 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <[email protected]>
Date: Tue, 6 Feb 2018 17:54:05 +0000
Subject: [PATCH] arm64: Allow user pointer tags to be passed into the kernel

The current tagged pointer ABI disallows the top byte of a user pointer to be non-zero when invoking a syscall. This patch allows such pointer to be passed into the kernel and the kernel will mask them out automatically. Page-based syscall ABI (mmap, mprotect, madvise etc.) expect the pointer tag to be 0 (see include/linux/syscalls.h for the ABI functions taking __user pointers).

Signed-off-by Catalin Marinas <[email protected]>
---
 arch/arm64/include/asm/unistd.h | 9 +++++++++
 include/linux/syscalls.h        | 2 ++
 2 files changed, 11 insertions(+)
arch/arm64/include/asm/unistd.h +9 -0
index a0baa9af5487..cd68ad969e3a 100644
@@ -53,3 +53,12 @@
 #endif
 
 #define NR_syscalls (__NR_syscalls)
+
+/* copied from arch/s390/ */
+#define __TYPE_IS_PTR(t) (!__builtin_types_compatible_p( \
+				typeof(0?(__force t)0:0ULL), u64))
+/* sign-extend bit 55 to mask out the pointer tag */
+#define __SC_CAST(t, a)						\
+	(__TYPE_IS_PTR(t)					\
+		? (__force t)((__s64)((__u64)a << 8) >> 8)	\
+		: (__force t)a)
include/linux/syscalls.h +2 -0
index a78186d826d7..279497207a31 100644
@@ -105,7 +105,9 @@ union bpf_attr;
 #define __TYPE_IS_UL(t)	(__TYPE_AS(t, 0UL))
 #define __TYPE_IS_LL(t) (__TYPE_AS(t, 0LL) || __TYPE_AS(t, 0ULL))
 #define __SC_LONG(t, a) __typeof(__builtin_choose_expr(__TYPE_IS_LL(t), 0LL, 0L)) a
+#ifndef __SC_CAST
 #define __SC_CAST(t, a)	(__force t) a
+#endif
 #define __SC_ARGS(t, a)	a
 #define __SC_TEST(t, a) (void)BUILD_BUG_ON_ZERO(!__TYPE_IS_LL(t) && sizeof(t) > sizeof(long))
Signature · 1 line
Catalin
author
#5 lore raw

Re: [RFC PATCH 2/6] arm64: untag user addresses in copy_from_user and others

Quoted 7 lines · Catalin Marinas
Quoted 6 lines · Mark Rutland
Quoted 4 lines · Andrey Konovalov

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.

Quoted 8 lines
Quoted 8 lines

I don't think it makes sense to do this in the low-level uaccess primitives, given we're going to have to untag pointers before common code can use them, e.g. for comparisons against TASK_SIZE or user_addr_max().

I think we'll end up with subtle bugs unless we consistently untag pointers before we get to uaccess primitives. If core code does untag pointers, then it's redundant to do so here.

There are two different approaches to untagging the user pointers that I see:

1. Untag user pointers right after they are passed to the kernel.

While this might be possible for pointers that are passed to syscalls as arguments (Catalin's "hack"), this leaves user pointers, that are embedded into for example structs that are passed to the kernel. Since there's no specification of the interface between user space and the kernel, different kernel parts handle user pointers differently and I don't see an easy way to cover them all.

2. Untag user pointers where they are used in the kernel.

Although there's no specification on the interface between the user space and the kernel, the kernel still has to use one of a few specific ways to access user data (copy_from_user, etc.). So the idea here is to add untagging into them. This patchset mostly takes this approach (with the exception of memory subsystem syscalls).

If there's a better approach, I'm open to suggestions.

author
#6 lore raw

RFC PATCH 3/6 mm, arm64: untag user addresses in memory syscalls

In reply to Andrey Konovalov

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 <[email protected]>
---
 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(+)
mm/madvise.c +2 -0
index 4d3c922ea1a1..909d6ba09031 100644
@@ -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;
mm/mempolicy.c +6 -0
index d879f1d8a44a..79d33a570c60 100644
@@ -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;
mm/mincore.c +2 -0
index fc37afe226e6..b59cf8fa3050 100644
@@ -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;
mm/mlock.c +5 -0
index 74e5a6547c3d..2f456a458cac 100644
@@ -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;
mm/mmap.c +9 -0
index 9efdc021ad22..b63362c45cde 100644
@@ -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);
mm/mprotect.c +2 -0
index e3309fcf586b..73d2a6befcf9 100644
@@ -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);
 }
mm/mremap.c +2 -0
index 049470aa1e3e..e42863a135de 100644
@@ -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;
mm/msync.c +3 -0
index ef30a429623a..03a977558f9f 100644
@@ -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;
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
maintainer
#7 lore raw

Re: [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls

Quoted 9 lines · Andrey Konovalov

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 <[email protected]>

Please keep the cc list small (maybe linux-arch, linux-arm-kernel) as I'm sure some lists would consider this spam.

Quoted 8 lines
 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 +++

I'm not yet convinced these functions need to allow tagged pointers. They are not doing memory accesses but rather dealing with the memory range, hence an untagged pointer is better suited. There is probably a reason why the "start" argument is "unsigned long" vs "void __user *" (in the kernel, not the man page).

Signature · 1 line
Catalin
author
#8 lore raw

Re: [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls

Quoted 5 lines · Catalin Marinas
Quoted 4 lines · Andrey Konovalov

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.

Quoted 8 lines
Quoted 5 lines

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 <[email protected]>

Please keep the cc list small (maybe linux-arch, linux-arm-kernel) as I'm sure some lists would consider this spam.

OK.

Quoted 10 lines
Quoted 9 lines
 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 +++
Quoted 5 lines

I'm not yet convinced these functions need to allow tagged pointers. They are not doing memory accesses but rather dealing with the memory range, hence an untagged pointer is better suited. There is probably a reason why the "start" argument is "unsigned long" vs "void __user *" (in the kernel, not the man page).

So that would make the user to untag pointers before passing to these syscalls.

Evgeniy, would that be possible to untag pointers in HWASan before using memory subsystem syscalls? Is there a reason for untagging them in the kernel?

Quoted 3 lines

--
Catalin

#9 lore raw

Re: [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls

Quoted 33 lines · Andrey Konovalov
Quoted 5 lines · Catalin Marinas
Quoted 4 lines · Andrey Konovalov

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.

Quoted 8 lines
Quoted 5 lines

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 <[email protected]>

Please keep the cc list small (maybe linux-arch, linux-arm-kernel) as I'm sure some lists would consider this spam.

OK.

Quoted 10 lines
Quoted 9 lines
 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 +++
Quoted 5 lines

I'm not yet convinced these functions need to allow tagged pointers. They are not doing memory accesses but rather dealing with the memory range, hence an untagged pointer is better suited. There is probably a reason why the "start" argument is "unsigned long" vs "void __user *" (in the kernel, not the man page).

Quoted 5 lines

So that would make the user to untag pointers before passing to these syscalls.

Evgeniy, would that be possible to untag pointers in HWASan before using memory subsystem syscalls? Is there a reason for untagging them in the kernel?

Generally, no. It's possible to intercept a libc call using symbol interposition, but I don't know how to rewrite arguments of a raw system call other than through ptrace, and that creates more problems than it solves.

AFAIU, it's valid for a program to pass an address obtained from malloc or, better, posix_memalign to an mm syscall like mprotect(). These arguments are pointers from the userspace point of view.

author
#10 lore raw

Re: [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls

Quoted 14 lines · Evgenii Stepanov
Quoted 7 lines · Andrey Konovalov
Quoted 5 lines · Catalin Marinas

I'm not yet convinced these functions need to allow tagged pointers. They are not doing memory accesses but rather dealing with the memory range, hence an untagged pointer is better suited. There is probably a reason why the "start" argument is "unsigned long" vs "void __user *" (in the kernel, not the man page).

Quoted 5 lines

So that would make the user to untag pointers before passing to these syscalls.

Evgeniy, would that be possible to untag pointers in HWASan before using memory subsystem syscalls? Is there a reason for untagging them in the kernel?

Quoted 8 lines

Generally, no. It's possible to intercept a libc call using symbol interposition, but I don't know how to rewrite arguments of a raw system call other than through ptrace, and that creates more problems than it solves.

AFAIU, it's valid for a program to pass an address obtained from malloc or, better, posix_memalign to an mm syscall like mprotect(). These arguments are pointers from the userspace point of view.

Catalin, do you think this is a good reason to have the untagging done in the kernel?

maintainer
#11 lore raw

Re: [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls

Quoted 11 lines · Andrey Konovalov
Quoted 10 lines · Evgenii Stepanov
Quoted 9 lines · Andrey Konovalov
Quoted 5 lines · Catalin Marinas

I'm not yet convinced these functions need to allow tagged pointers. They are not doing memory accesses but rather dealing with the memory range, hence an untagged pointer is better suited. There is probably a reason why the "start" argument is "unsigned long" vs "void __user *" (in the kernel, not the man page).

So that would make the user to untag pointers before passing to these syscalls.

Quoted 8 lines
Quoted 8 lines
Quoted 3 lines

Evgeniy, would that be possible to untag pointers in HWASan before using memory subsystem syscalls? Is there a reason for untagging them in the kernel?

Generally, no. It's possible to intercept a libc call using symbol interposition, but I don't know how to rewrite arguments of a raw system call other than through ptrace, and that creates more problems than it solves.

With these patches, we are trying to relax the user/kernel ABI so that tagged pointers can be passed into the kernel. Since this is a new ABI (or an extension to the existing one), it might be ok to change the libc so that the top byte is zeroed on specific syscalls before issuing the SVC.

I agree that it is problematic for HWASan if it only relies on overriding malloc/free.

Quoted 6 lines
Quoted 3 lines

AFAIU, it's valid for a program to pass an address obtained from malloc or, better, posix_memalign to an mm syscall like mprotect(). These arguments are pointers from the userspace point of view.

Catalin, do you think this is a good reason to have the untagging done in the kernel?

malloc() or posix_memalign() are C library implementations and it's the C library (or overridden functions) setting a tag on the returned pointers. Since the TBI hardware feature allows memory accesses with a non-zero tag, we could allow them in the kernel for syscalls performing such accesses on behalf of the user (e.g. get_user/put_user would not need to clear the tag).

madvise(), OTOH, does not perform a memory access on behalf of the user, it's just advising the kernel about a range of virtual addresses. That's where I think, from an ABI perspective, it doesn't make much sense to allow tags into the kernel for these syscalls (even if it's simpler from a user space perspective).

(but I don't have a very strong opinion on this ;))

Signature · 1 line
Catalin
#12 lore raw

Re: [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls

Quoted 44 lines · Catalin Marinas
Quoted 11 lines · Andrey Konovalov
Quoted 10 lines · Evgenii Stepanov
Quoted 9 lines · Andrey Konovalov
Quoted 5 lines · Catalin Marinas

I'm not yet convinced these functions need to allow tagged pointers. They are not doing memory accesses but rather dealing with the memory range, hence an untagged pointer is better suited. There is probably a reason why the "start" argument is "unsigned long" vs "void __user *" (in the kernel, not the man page).

So that would make the user to untag pointers before passing to these syscalls.

Quoted 8 lines
Quoted 8 lines
Quoted 3 lines

Evgeniy, would that be possible to untag pointers in HWASan before using memory subsystem syscalls? Is there a reason for untagging them in the kernel?

Generally, no. It's possible to intercept a libc call using symbol interposition, but I don't know how to rewrite arguments of a raw system call other than through ptrace, and that creates more problems than it solves.

With these patches, we are trying to relax the user/kernel ABI so that tagged pointers can be passed into the kernel. Since this is a new ABI (or an extension to the existing one), it might be ok to change the libc so that the top byte is zeroed on specific syscalls before issuing the SVC.

I agree that it is problematic for HWASan if it only relies on overriding malloc/free.

Quoted 6 lines
Quoted 3 lines

AFAIU, it's valid for a program to pass an address obtained from malloc or, better, posix_memalign to an mm syscall like mprotect(). These arguments are pointers from the userspace point of view.

Catalin, do you think this is a good reason to have the untagging done in the kernel?

malloc() or posix_memalign() are C library implementations and it's the C library (or overridden functions) setting a tag on the returned pointers. Since the TBI hardware feature allows memory accesses with a non-zero tag, we could allow them in the kernel for syscalls performing such accesses on behalf of the user (e.g. get_user/put_user would not need to clear the tag).

Quoted 7 lines

madvise(), OTOH, does not perform a memory access on behalf of the user, it's just advising the kernel about a range of virtual addresses. That's where I think, from an ABI perspective, it doesn't make much sense to allow tags into the kernel for these syscalls (even if it's simpler from a user space perspective).

(but I don't have a very strong opinion on this ;))

I don't have a strong opinion on this, either.
Ideally, I would like tags to be fully transparent for user space code. MM syscalls used on a malloc/memalign address are not a very common pattern, so it might be OK to not allow tags there. But all such code will have to be changed with explicit knowledge of TBI.

author
#13 lore raw

RFC PATCH 4/6 mm, arm64: untag user addresses in mm/gup.c

In reply to Andrey Konovalov

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 <[email protected]>
---
 mm/gup.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
mm/gup.c +12 -0
index 1b46e6e74881..4d820c4792d7 100644
@@ -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;
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
author
#14 lore raw

RFC PATCH 5/6 lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user

In reply to Andrey Konovalov

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 <[email protected]>
---
 lib/strncpy_from_user.c | 2 ++
 lib/strnlen_user.c      | 2 ++
 2 files changed, 4 insertions(+)
lib/strncpy_from_user.c +2 -0
index b53e1b5d80f4..97467cd2bc59 100644
@@ -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)) {
lib/strnlen_user.c +2 -0
index 60d0bbda8f5e..8b5f56466e00 100644
@@ -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)) {
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
author
#15 lore raw

RFC PATCH 6/6 arch: add untagged_addr definition for other arches

In reply to Andrey Konovalov

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 <[email protected]>
---
 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(+)
arch/alpha/include/asm/uaccess.h +2 -0
index 87d8c4f0307d..09d136bb4ff5 100644
@@ -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
arch/arc/include/asm/uaccess.h +1 -0
index c9173c02081c..2a04b7a4aada 100644
@@ -26,6 +26,7 @@
 
 #include <linux/string.h>	/* for generic string functions */
 
+#define untagged_addr(addr)	addr
 
 #define __kernel_ok		(uaccess_kernel())
arch/arm/include/asm/uaccess.h +2 -0
index 0bf2347495f1..7d4f4e4021f2 100644
@@ -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
arch/blackfin/include/asm/uaccess.h +2 -0
index 45da4bcb050e..fb6bdc54e7bd 100644
@@ -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)
arch/c6x/include/asm/uaccess.h +2 -0
index ba6756879f00..f187696cf440 100644
@@ -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>
arch/cris/include/asm/uaccess.h +2 -0
index 3b42ab0cae93..86d8fbd200c4 100644
@@ -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
arch/frv/include/asm/uaccess.h +2 -0
index ff9562dc6825..be21b42bde09 100644
@@ -12,6 +12,8 @@
 #ifndef _ASM_UACCESS_H
 #define _ASM_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 /*
  * User space memory access functions
  */
arch/ia64/include/asm/uaccess.h +2 -0
index a74524f2d625..1c46bf1c4f73 100644
@@ -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:
  */
arch/m32r/include/asm/uaccess.h +2 -0
index 9d89bc3d8181..6e0fe6b215be 100644
@@ -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
arch/m68k/include/asm/uaccess.h +2 -0
index e896466a41a4..02e0c5878ad5 100644
@@ -5,3 +5,5 @@
 #include <asm/uaccess_mm.h>
 #endif
 #include <asm/extable.h>
+
+#define untagged_addr(addr)	addr
arch/metag/include/asm/uaccess.h +2 -0
index a5311eb36e32..1b2f0478868a 100644
@@ -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)
arch/microblaze/include/asm/uaccess.h +2 -0
index 81f16aadbf9e..a66bc26660c3 100644
@@ -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.
arch/mips/include/asm/uaccess.h +2 -0
index b71306947290..2db7606c388b 100644
@@ -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
arch/mn10300/include/asm/uaccess.h +2 -0
index 5af468fd1359..6604699b34b6 100644
@@ -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
arch/nios2/include/asm/uaccess.h +2 -0
index dfa3c7cb30b4..36152a7302a8 100644
@@ -19,6 +19,8 @@
 
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * Segment stuff
  */
arch/openrisc/include/asm/uaccess.h +2 -0
index bbf5c79cce7a..5b43d13ab363 100644
@@ -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
arch/parisc/include/asm/uaccess.h +2 -0
index ea70e36ce6af..b0f3cd529c8d 100644
@@ -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})
arch/powerpc/include/asm/uaccess.h +2 -0
index 51bfeb8777f0..07ae1c318166 100644
@@ -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
arch/riscv/include/asm/uaccess.h +2 -0
index 14b0b22fb578..e774239aac24 100644
@@ -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()							\
arch/score/include/asm/uaccess.h +2 -0
index a233f3236846..fd16c2a71091 100644
@@ -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)
arch/sh/include/asm/uaccess.h +2 -0
index 32eb56e00c11..31f3ea075190 100644
@@ -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)
arch/sparc/include/asm/uaccess.h +2 -0
index dd85bc2c2cad..70c2f5ea09ce 100644
@@ -7,6 +7,8 @@
 #include <asm/uaccess_32.h>
 #endif
 
+#define untagged_addr(addr)	addr
+
 #define user_addr_max() \
 	(uaccess_kernel() ? ~0UL : TASK_SIZE)
arch/tile/include/asm/uaccess.h +2 -0
index cb4fbe7e4f88..7d365b087dcb 100644
@@ -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
arch/x86/include/asm/uaccess.h +2 -0
index aae77eb8491c..3c233fbdd32b 100644
@@ -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
arch/xtensa/include/asm/uaccess.h +2 -0
index f1158b4c629c..130e419c4d6e 100644
@@ -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
include/asm-generic/uaccess.h +2 -0
index 6b2e63df2739..2c46d2253dba 100644
@@ -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))
 
 /*
Signature · 1 line
2.16.2.395.g2e18187dfd-goog
maintainer
#16 lore raw

Re: [RFC PATCH 6/6] arch: add untagged_addr definition for other arches

Quoted 28 lines · Andrey Konovalov

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 <[email protected]>
---
 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 ++
Quoted 8 lines
 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

#17 lore raw

Re: [RFC PATCH 6/6] arch: add untagged_addr definition for other arches

In reply to Andrey Konovalov
Quoted 4 lines · Andrey Konovalov

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.

Quoted 367 lines

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

Signed-off-by Andrey Konovalov <[email protected]>
---
  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(+)
arch/alpha/include/asm/uaccess.h +2 -0
index 87d8c4f0307d..09d136bb4ff5 100644
@@ -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
arch/arc/include/asm/uaccess.h +1 -0
index c9173c02081c..2a04b7a4aada 100644
@@ -26,6 +26,7 @@
 
  #include <linux/string.h>	/* for generic string functions */
 
+#define untagged_addr(addr)	addr
 
  #define __kernel_ok		(uaccess_kernel())
arch/arm/include/asm/uaccess.h +2 -0
index 0bf2347495f1..7d4f4e4021f2 100644
@@ -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
arch/blackfin/include/asm/uaccess.h +2 -0
index 45da4bcb050e..fb6bdc54e7bd 100644
@@ -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)
arch/c6x/include/asm/uaccess.h +2 -0
index ba6756879f00..f187696cf440 100644
@@ -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>
arch/cris/include/asm/uaccess.h +2 -0
index 3b42ab0cae93..86d8fbd200c4 100644
@@ -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
arch/frv/include/asm/uaccess.h +2 -0
index ff9562dc6825..be21b42bde09 100644
@@ -12,6 +12,8 @@
  #ifndef _ASM_UACCESS_H
  #define _ASM_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
  /*
   * User space memory access functions
   */
arch/ia64/include/asm/uaccess.h +2 -0
index a74524f2d625..1c46bf1c4f73 100644
@@ -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:
   */
arch/m32r/include/asm/uaccess.h +2 -0
index 9d89bc3d8181..6e0fe6b215be 100644
@@ -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
arch/m68k/include/asm/uaccess.h +2 -0
index e896466a41a4..02e0c5878ad5 100644
@@ -5,3 +5,5 @@
  #include <asm/uaccess_mm.h>
  #endif
  #include <asm/extable.h>
+
+#define untagged_addr(addr)	addr
arch/metag/include/asm/uaccess.h +2 -0
index a5311eb36e32..1b2f0478868a 100644
@@ -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)
arch/microblaze/include/asm/uaccess.h +2 -0
index 81f16aadbf9e..a66bc26660c3 100644
@@ -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.
arch/mips/include/asm/uaccess.h +2 -0
index b71306947290..2db7606c388b 100644
@@ -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
arch/mn10300/include/asm/uaccess.h +2 -0
index 5af468fd1359..6604699b34b6 100644
@@ -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
arch/nios2/include/asm/uaccess.h +2 -0
index dfa3c7cb30b4..36152a7302a8 100644
@@ -19,6 +19,8 @@
 
  #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
  /*
   * Segment stuff
   */
arch/openrisc/include/asm/uaccess.h +2 -0
index bbf5c79cce7a..5b43d13ab363 100644
@@ -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
arch/parisc/include/asm/uaccess.h +2 -0
index ea70e36ce6af..b0f3cd529c8d 100644
@@ -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})
arch/powerpc/include/asm/uaccess.h +2 -0
index 51bfeb8777f0..07ae1c318166 100644
@@ -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
arch/riscv/include/asm/uaccess.h +2 -0
index 14b0b22fb578..e774239aac24 100644
@@ -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()							\
arch/score/include/asm/uaccess.h +2 -0
index a233f3236846..fd16c2a71091 100644
@@ -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)
arch/sh/include/asm/uaccess.h +2 -0
index 32eb56e00c11..31f3ea075190 100644
@@ -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)
arch/sparc/include/asm/uaccess.h +2 -0
index dd85bc2c2cad..70c2f5ea09ce 100644
@@ -7,6 +7,8 @@
  #include <asm/uaccess_32.h>
  #endif
 
+#define untagged_addr(addr)	addr
+
  #define user_addr_max() \
  	(uaccess_kernel() ? ~0UL : TASK_SIZE)
arch/tile/include/asm/uaccess.h +2 -0
index cb4fbe7e4f88..7d365b087dcb 100644
@@ -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
arch/x86/include/asm/uaccess.h +2 -0
index aae77eb8491c..3c233fbdd32b 100644
@@ -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
arch/xtensa/include/asm/uaccess.h +2 -0
index f1158b4c629c..130e419c4d6e 100644
@@ -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
include/asm-generic/uaccess.h +2 -0
index 6b2e63df2739..2c46d2253dba 100644
@@ -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))
 
  /*
author
#18 lore raw

Re: [RFC PATCH 6/6] arch: add untagged_addr definition for other arches

Quoted 11 lines · Robin Murphy
Quoted 5 lines · Andrey Konovalov

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!

#19 lore raw
In reply to Andrey Konovalov

Hi Andrey,

Quoted 4 lines · Andrey Konovalov

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.

Quoted 63 lines

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(-)
author
#20 lore raw
Quoted 10 lines · Robin Murphy

Hi Andrey,

Quoted 5 lines · Andrey Konovalov

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.

Quoted 5 lines

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!

reviewer
#21 lore raw
In reply to Andrey Konovalov

Hi,

[trimming Ccs]

Quoted 5 lines · Andrey Konovalov

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.

Quoted 7 lines

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.

There are many ways that user pointers can be passed to the kernel, and I'm not sure that it's feasible to catch them all, especially as user pointers are often passed in data structures (e.g. iovecs) rather than direct syscall arguments.

If we *really* want the kernel to support taking tagged addresses, anything with a __user annotation (or cast to something with a __user annotation) requires tag removal somewhere in the kernel.

It looks like there are plenty uapi structures and syscalls to look at:

[mark@lakrids:~/src/linux]% git grep __user -- include/uapi | wc -l 216
[mark@lakrids:~/src/linux]% git grep __user | grep SYSCALL_DEFINE | wc -l 308

... in addition to special syscalls like ioctl which multiplex a number of operations with different arguments, where the tag stripping would have to occur elsewhere (e.g. in particular drivers).

I also wonder if we ever write any of these pointers back to userspace memory. If so, we have a nasty ABI problem, since we'll have to marshal the original tag along with the pointer, to ensure userspace pointer comparisons continue to work.

Thanks,
Mark.

maintainer
#22 lore raw
Quoted 25 lines · Mark Rutland
Quoted 5 lines · Andrey Konovalov

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.

Quoted 7 lines

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.

There are many ways that user pointers can be passed to the kernel, and I'm not sure that it's feasible to catch them all, especially as user pointers are often passed in data structures (e.g. iovecs) rather than direct syscall arguments.

If we *really* want the kernel to support taking tagged addresses, anything with a __user annotation (or cast to something with a __user annotation) requires tag removal somewhere in the kernel.

It looks like there are plenty uapi structures and syscalls to look at:

Quoted 8 lines

[mark@lakrids:~/src/linux]% git grep __user -- include/uapi | wc -l 216
[mark@lakrids:~/src/linux]% git grep __user | grep SYSCALL_DEFINE | wc -l 308

... in addition to special syscalls like ioctl which multiplex a number of operations with different arguments, where the tag stripping would have to occur elsewhere (e.g. in particular drivers).

Hence we definitely need good support from e.g. sparse to catch any errors from happening.

So doing assignments to the same pointer variable like

     addr = untagged_addr(addr);

defeats any checking.

Quoted 4 lines

I also wonder if we ever write any of these pointers back to userspace memory. If so, we have a nasty ABI problem, since we'll have to marshal the original tag along with the pointer, to ensure userspace pointer comparisons continue to work.

Oh, another can of worms...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author
#23 lore raw
In reply to Mark Rutland
Quoted 38 lines · Mark Rutland

Hi,

[trimming Ccs]

Quoted 5 lines · Andrey Konovalov

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.

Quoted 7 lines

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.

There are many ways that user pointers can be passed to the kernel, and I'm not sure that it's feasible to catch them all, especially as user pointers are often passed in data structures (e.g. iovecs) rather than direct syscall arguments.

If we *really* want the kernel to support taking tagged addresses, anything with a __user annotation (or cast to something with a __user annotation) requires tag removal somewhere in the kernel.

It looks like there are plenty uapi structures and syscalls to look at:

[mark@lakrids:~/src/linux]% git grep __user -- include/uapi | wc -l 216
[mark@lakrids:~/src/linux]% git grep __user | grep SYSCALL_DEFINE | wc -l 308

... in addition to special syscalls like ioctl which multiplex a number of operations with different arguments, where the tag stripping would have to occur elsewhere (e.g. in particular drivers).

Quoted 7 lines

I also wonder if we ever write any of these pointers back to userspace memory. If so, we have a nasty ABI problem, since we'll have to marshal the original tag along with the pointer, to ensure userspace pointer comparisons continue to work.

Thanks,
Mark.

Hi Mark!

This seems to be similar to what you said in reply to one of the other patches, replied there.

Thanks!