Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes

From: Mathieu Desnoyers
Date: Fri Jul 06 2018 - 12:03:03 EST

Next message: Dan Williams: "Re: [PATCH] x86/numa_emulation: Fix uniform size build failure"
Previous message: Will Deacon: "Re: [PATCH] arm64: add ARM64-specific support for flatmem"
In reply to: Mathieu Desnoyers: "[RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes"
Next in thread: Mathieu Desnoyers: "Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

----- On Jul 5, 2018, at 2:05 PM, Mathieu Desnoyers mathieu.desnoyers@xxxxxxxxxxxx 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 <mathieu.desnoyers@xxxxxxxxxxxx>
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 <mathieu.desnoyers@xxxxxxxxxxxx>
CC: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>
CC: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
CC: Paul Turner <pjt@xxxxxxxxxx>
CC: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
CC: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
CC: Andi Kleen <andi@xxxxxxxxxxxxxx>
CC: Dave Watson <davejwatson@xxxxxx>
CC: Chris Lameter <cl@xxxxxxxxx>
CC: Ingo Molnar <mingo@xxxxxxxxxx>
CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
CC: Ben Maurer <bmaurer@xxxxxx>
CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
CC: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
CC: Russell King <linux@xxxxxxxxxxxxxxxx>
CC: Catalin Marinas <catalin.marinas@xxxxxxx>
CC: Will Deacon <will.deacon@xxxxxxx>
CC: Michael Kerrisk <mtk.manpages@xxxxxxxxx>
CC: Boqun Feng <boqun.feng@xxxxxxxxx>
CC: linux-api@xxxxxxxxxxxxxxx

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) \

>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> CC: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>
> CC: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> CC: Paul Turner <pjt@xxxxxxxxxx>
> CC: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> CC: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> CC: Andi Kleen <andi@xxxxxxxxxxxxxx>
> CC: Dave Watson <davejwatson@xxxxxx>
> CC: Chris Lameter <cl@xxxxxxxxx>
> CC: Ingo Molnar <mingo@xxxxxxxxxx>
> CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
> CC: Ben Maurer <bmaurer@xxxxxx>
> CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
> CC: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> CC: Russell King <linux@xxxxxxxxxxxxxxxx>
> CC: Catalin Marinas <catalin.marinas@xxxxxxx>
> CC: Will Deacon <will.deacon@xxxxxxx>
> CC: Michael Kerrisk <mtk.manpages@xxxxxxxxx>
> CC: Boqun Feng <boqun.feng@xxxxxxxxx>
> CC: linux-api@xxxxxxxxxxxxxxx
> ---
> 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@xxxxxxxxxxxx>
> */
>
> -#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.h
> b/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

Next message: Dan Williams: "Re: [PATCH] x86/numa_emulation: Fix uniform size build failure"
Previous message: Will Deacon: "Re: [PATCH] arm64: add ARM64-specific support for flatmem"
In reply to: Mathieu Desnoyers: "[RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes"
Next in thread: Mathieu Desnoyers: "Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]