----- On Jul 5, 2018, at 2:05 PM, Mathieu Desnoyers 
mathieu.desnoy...@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 <mathieu.desnoy...@efficios.com>
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.desnoy...@efficios.com>
    CC: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
    CC: Peter Zijlstra <pet...@infradead.org>
    CC: Paul Turner <p...@google.com>
    CC: Thomas Gleixner <t...@linutronix.de>
    CC: Andy Lutomirski <l...@amacapital.net>
    CC: Andi Kleen <a...@firstfloor.org>
    CC: Dave Watson <davejwat...@fb.com>
    CC: Chris Lameter <c...@linux.com>
    CC: Ingo Molnar <mi...@redhat.com>
    CC: "H. Peter Anvin" <h...@zytor.com>
    CC: Ben Maurer <bmau...@fb.com>
    CC: Steven Rostedt <rost...@goodmis.org>
    CC: Josh Triplett <j...@joshtriplett.org>
    CC: Linus Torvalds <torva...@linux-foundation.org>
    CC: Andrew Morton <a...@linux-foundation.org>
    CC: Russell King <li...@arm.linux.org.uk>
    CC: Catalin Marinas <catalin.mari...@arm.com>
    CC: Will Deacon <will.dea...@arm.com>
    CC: Michael Kerrisk <mtk.manpa...@gmail.com>
    CC: Boqun Feng <boqun.f...@gmail.com>
    CC: linux-...@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)                    \


> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoy...@efficios.com>
> CC: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
> CC: Peter Zijlstra <pet...@infradead.org>
> CC: Paul Turner <p...@google.com>
> CC: Thomas Gleixner <t...@linutronix.de>
> CC: Andy Lutomirski <l...@amacapital.net>
> CC: Andi Kleen <a...@firstfloor.org>
> CC: Dave Watson <davejwat...@fb.com>
> CC: Chris Lameter <c...@linux.com>
> CC: Ingo Molnar <mi...@redhat.com>
> CC: "H. Peter Anvin" <h...@zytor.com>
> CC: Ben Maurer <bmau...@fb.com>
> CC: Steven Rostedt <rost...@goodmis.org>
> CC: Josh Triplett <j...@joshtriplett.org>
> CC: Linus Torvalds <torva...@linux-foundation.org>
> CC: Andrew Morton <a...@linux-foundation.org>
> CC: Russell King <li...@arm.linux.org.uk>
> CC: Catalin Marinas <catalin.mari...@arm.com>
> CC: Will Deacon <will.dea...@arm.com>
> CC: Michael Kerrisk <mtk.manpa...@gmail.com>
> CC: Boqun Feng <boqun.f...@gmail.com>
> CC: linux-...@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.desnoy...@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.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

Reply via email to