arm64: untag user pointers passed to the kernel

RFC PATCH Superseded Andrey Konovalov 1 participant 12 messages Mar 27, 2018

Subsystems (5)

Memory management Documentation Library code arm64 +1 more subsystems

lore

Share & tools

lore

Andrey Konovalov

author

2018-03-27 16:57 UTC #0 lore raw

v2 cover arm64: untag user pointers passed to the kernel

Hi!

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.

Thanks!

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

Changes in RFC v2:
- Added "#ifndef untagged_addr..." fallback in linux/uaccess.h instead of defining it for each arch individually.
- Updated Documentation/arm64/tagged-pointers.txt.
- Dropped ?mm, arm64: untag user addresses in memory syscalls?.
- Rebased onto 3eb2ce82 (4.16-rc7).

Andrey Konovalov (6):
arm64: add type casts to untagged_addr macro
uaccess: add untagged_addr definition for other arches
arm64: untag user addresses in copy_from_user and others mm, arm64: untag user addresses in mm/gup.c
lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
arm64: update Documentation/arm64/tagged-pointers.txt

 Documentation/arm64/tagged-pointers.txt |  5 +++--
 arch/arm64/include/asm/uaccess.h        |  9 +++++++--
 include/linux/uaccess.h                 |  4 ++++
 lib/strncpy_from_user.c                 |  2 ++
 lib/strnlen_user.c                      |  2 ++
 mm/gup.c                                | 12 ++++++++++++
 6 files changed, 30 insertions(+), 4 deletions(-)

Signature · 1 line

2.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #1 lore raw

RFC PATCH v2 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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #2 lore raw

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

In reply to Andrey Konovalov

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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #3 lore raw

RFC PATCH v2 2/6 uaccess: 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 other architectures besides arm64.

Signed-off-by Andrey Konovalov <[email protected]>

---
 include/linux/uaccess.h | 4 ++++
 1 file changed, 4 insertions(+)

include/linux/uaccess.h +4 -0

index efe79c1cdd47..c045b4eff95e 100644
@@ -13,6 +13,10 @@
 
 #include <asm/uaccess.h>
 
+#ifndef untagged_addr
+#define untagged_addr(addr) addr
+#endif
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and

Signature · 1 line

2.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #4 lore raw

RFC PATCH v2 2/6 uaccess: add untagged_addr definition for other arches

In reply to Andrey Konovalov

Define it as a noop for other architectures besides arm64.

Signed-off-by Andrey Konovalov <[email protected]>

---
 include/linux/uaccess.h | 4 ++++
 1 file changed, 4 insertions(+)

include/linux/uaccess.h +4 -0

index efe79c1cdd47..c045b4eff95e 100644
@@ -13,6 +13,10 @@
 
 #include <asm/uaccess.h>
 
+#ifndef untagged_addr
+#define untagged_addr(addr) addr
+#endif
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and

Signature · 1 line

2.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #5 lore raw

RFC PATCH v2 3/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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #6 lore raw

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

In reply to Andrey Konovalov

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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #7 lore raw

RFC PATCH v2 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 6afae32571ca..9c4afcf50dfa 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);

@@ -1749,6 +1757,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;

@@ -1801,6 +1811,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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #8 lore raw

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

In reply to Andrey Konovalov

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 6afae32571ca..9c4afcf50dfa 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);

@@ -1749,6 +1757,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;

@@ -1801,6 +1811,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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #9 lore raw

RFC PATCH v2 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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #10 lore raw

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

In reply to Andrey Konovalov

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.17.0.rc0.231.g781580f067-goog

Andrey Konovalov

author

2018-03-27 16:57 UTC #11 lore raw

RFC PATCH v2 6/6 arm64: update Documentation/arm64/tagged-pointers.txt

In reply to Andrey Konovalov

Add a note that work on passing tagged user pointers to the kernel via syscalls has started, but might not be complete yet.

Signed-off-by Andrey Konovalov <[email protected]>

---
 Documentation/arm64/tagged-pointers.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Documentation/arm64/tagged-pointers.txt +3 -2

index a25a99e82bb1..361481283f00 100644
@@ -35,8 +35,9 @@ Using non-zero address tags in any of these locations may result in an
 error code being returned, a (fatal) signal being raised, or other modes
 of failure.
 
-For these reasons, passing non-zero address tags to the kernel via
-system calls is forbidden, and using a non-zero address tag for sp is
+Some initial work for supporting non-zero address tags passed to the
+kernel via system calls has been done, but the kernel doesn't provide
+any guarantees at this point. Using a non-zero address tag for sp is
 strongly discouraged.
 
 Programs maintaining a frame pointer and frame records that use non-zero

Signature · 1 line

2.17.0.rc0.231.g781580f067-goog