Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: 2018-07-06 16:03:03
Also in:
lkml
Subsystem:
arm port, the rest · Maintainers:
Russell King, Linus Torvalds
----- On Jul 5, 2018, at 2:05 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
Declaring the rseq_cs field as a union between __u64 and two __u32 allows both 32-bit and 64-bit kernels to read the full __u64, and therefore validate that a 32-bit user-space cleared the upper 32 bits, thus ensuring a consistent behavior between native 32-bit kernels and 32-bit compat tasks on 64-bit kernels. Check that the rseq_cs value read is < TASK_SIZE. The asm/byteorder.h header needs to be included by rseq.h, now that it is not using linux/types_32_64.h anymore. Considering that only __32 and __u64 types are declared in linux/rseq.h, the linux/types.h header should always be included for both kernel and user-space code: including stdint.h is just for u64 and u32, which are not used in this header at all.
The 0-day bot noticed that __get_user() is unimplemented for 64-bit
values on arm32 (although get_user() is implemented).
The following diff fixes this discrepancy, and allows this rseq patch
to build on arm32:
commit dde99f3310c76acb0a160c0572f40b6aa279594c
Author: Mathieu Desnoyers [off-list ref]
Date: Fri Jul 6 11:29:39 2018 -0400
arm: implement 64-bit __get_user
get_user() is implemented on arm32 for 64-bit user-space values, but
not its __get_user() counterpart.
Implement __get_user() as two __get_user_asm_word().
Signed-off-by: Mathieu Desnoyers [off-list ref]
CC: "Paul E. McKenney" [off-list ref]
CC: Peter Zijlstra [off-list ref]
CC: Paul Turner [off-list ref]
CC: Thomas Gleixner [off-list ref]
CC: Andy Lutomirski [off-list ref]
CC: Andi Kleen [off-list ref]
CC: Dave Watson [off-list ref]
CC: Chris Lameter [off-list ref]
CC: Ingo Molnar [off-list ref]
CC: "H. Peter Anvin" [off-list ref]
CC: Ben Maurer [off-list ref]
CC: Steven Rostedt [off-list ref]
CC: Josh Triplett [off-list ref]
CC: Linus Torvalds [off-list ref]
CC: Andrew Morton [off-list ref]
CC: Russell King [off-list ref]
CC: Catalin Marinas [off-list ref]
CC: Will Deacon [off-list ref]
CC: Michael Kerrisk [off-list ref]
CC: Boqun Feng [off-list ref]
CC: linux-api@vger.kernel.org
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 3d614e9..38659c6 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h@@ -265,10 +265,16 @@ static inline void set_fs(mm_segment_t fs) (void) 0; \ }) +union __gu_u64 { + u64 val64; + u32 word[2]; +}; + #define __get_user_err(x, ptr, err) \ do { \ unsigned long __gu_addr = (unsigned long)(ptr); \ unsigned long __gu_val; \ + union __gu_u64 __gu_tmp64; \ unsigned int __ua_flags; \ __chk_user_ptr(ptr); \ might_fault(); \
@@ -277,10 +283,28 @@ static inline void set_fs(mm_segment_t fs) case 1: __get_user_asm_byte(__gu_val, __gu_addr, err); break; \ case 2: __get_user_asm_half(__gu_val, __gu_addr, err); break; \ case 4: __get_user_asm_word(__gu_val, __gu_addr, err); break; \ + case 8: \ + { \ + union __gu_u64 __user *__gu_addr64 = \ + (union __gu_u64 __user *)__gu_addr; \ + __get_user_asm_word(__gu_tmp64.word[0], \ + &__gu_addr64->word[0], err); \ + if (err) \ + break; \ + __get_user_asm_word(__gu_tmp64.word[1], \ + &__gu_addr64->word[1], err); \ + break; \ + }; \ default: (__gu_val) = __get_user_bad(); \ } \ uaccess_restore(__ua_flags); \ - (x) = (__typeof__(*(ptr)))__gu_val; \ + switch (sizeof(*(ptr))) { \ + case 1: \ + case 2: \ + case 4: \ + default: (x) = (__typeof__(*(ptr)))__gu_val; break; \ + case 8: (x) = (__typeof__(*(ptr)))__gu_tmp64.val64; break; \ + } \ } while (0) #define __get_user_asm(x, addr, err, instr) \
quoted hunk ↗ jump to hunk
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> CC: "Paul E. McKenney" <redacted> CC: Peter Zijlstra <peterz@infradead.org> CC: Paul Turner <redacted> CC: Thomas Gleixner <redacted> CC: Andy Lutomirski <luto@amacapital.net> CC: Andi Kleen <redacted> CC: Dave Watson <redacted> CC: Chris Lameter <redacted> CC: Ingo Molnar <mingo@redhat.com> CC: "H. Peter Anvin" <hpa@zytor.com> CC: Ben Maurer <redacted> CC: Steven Rostedt <rostedt@goodmis.org> CC: Josh Triplett <josh@joshtriplett.org> CC: Linus Torvalds <torvalds@linux-foundation.org> CC: Andrew Morton <akpm@linux-foundation.org> CC: Russell King <redacted> CC: Catalin Marinas <catalin.marinas@arm.com> CC: Will Deacon <redacted> CC: Michael Kerrisk <redacted> CC: Boqun Feng <redacted> CC: linux-api@vger.kernel.org --- include/uapi/linux/rseq.h | 27 +++++++++++++++++++-------- kernel/rseq.c | 12 +++++++----- tools/testing/selftests/rseq/rseq.h | 11 ++++++++++- 3 files changed, 36 insertions(+), 14 deletions(-)diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h index bf4188c13bec..9a402fdb60e9 100644 --- a/include/uapi/linux/rseq.h +++ b/include/uapi/linux/rseq.h@@ -10,13 +10,8 @@ * Copyright (c) 2015-2018 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> */ -#ifdef __KERNEL__ -# include <linux/types.h> -#else -# include <stdint.h> -#endif - -#include <linux/types_32_64.h> +#include <linux/types.h> +#include <asm/byteorder.h>enum rseq_cpu_id_state { RSEQ_CPU_ID_UNINITIALIZED = -1,@@ -111,7 +106,23 @@ struct rseq {* atomicity semantics. This field should only be updated by the * thread which registered this data structure. Aligned on 64-bit. */ - LINUX_FIELD_u32_u64(rseq_cs); + union { + __u64 ptr64; +#ifdef __LP64__ + __u64 ptr; +#else + struct { +#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || defined(__BIG_ENDIAN) + __u32 padding; /* Initialized to zero. */ + __u32 ptr32; +#else /* LITTLE */ + __u32 ptr32; + __u32 padding; /* Initialized to zero. */ +#endif /* ENDIAN */ + } ptr; +#endif + } rseq_cs; + /* * Restartable sequences flags field. *diff --git a/kernel/rseq.c b/kernel/rseq.c index 16b38c5342f9..3081e6783cce 100644 --- a/kernel/rseq.c +++ b/kernel/rseq.c@@ -115,19 +115,21 @@ static int rseq_reset_rseq_cpu_id(struct task_struct *t)static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs) { struct rseq_cs __user *urseq_cs; - unsigned long ptr; + u64 ptr; u32 __user *usig; u32 sig; int ret; - ret = __get_user(ptr, &t->rseq->rseq_cs); + ret = __get_user(ptr, &t->rseq->rseq_cs.ptr64); if (ret) return ret; if (!ptr) { memset(rseq_cs, 0, sizeof(*rseq_cs)); return 0; } - urseq_cs = (struct rseq_cs __user *)ptr; + if (ptr >= TASK_SIZE) + return -EINVAL; + urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr; if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs))) return -EFAULT;@@ -201,9 +203,9 @@ static int clear_rseq_cs(struct task_struct *t)* of code outside of the rseq assembly block. This performs * a lazy clear of the rseq_cs field. * - * Set rseq_cs to NULL with single-copy atomicity. + * Set rseq_cs to NULL. */ - return __put_user(0UL, &t->rseq->rseq_cs); + return __put_user(0ULL, &t->rseq->rseq_cs.ptr64); } /*diff --git a/tools/testing/selftests/rseq/rseq.hb/tools/testing/selftests/rseq/rseq.h index a4684112676c..f2073cfa4448 100644--- a/tools/testing/selftests/rseq/rseq.h +++ b/tools/testing/selftests/rseq/rseq.h@@ -133,6 +133,15 @@ static inline uint32_t rseq_current_cpu(void)return cpu; } +static inline void rseq_clear_rseq_cs(void) +{ +#ifdef __LP64__ + __rseq_abi.rseq_cs.ptr = 0; +#else + __rseq_abi.rseq_cs.ptr.ptr32 = 0; +#endif +} + /* * rseq_prepare_unload() should be invoked by each thread using rseq_finish*() * at least once between their last rseq_finish*() and library unload of the@@ -143,7 +152,7 @@ static inline uint32_t rseq_current_cpu(void) */static inline void rseq_prepare_unload(void) { - __rseq_abi.rseq_cs = 0; + rseq_clear_rseq_cs(); } #endif /* RSEQ_H_ */ -- 2.11.0
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com