|
|
Message-ID: <1477407767.2263.22.camel@cvidal.org>
Date: Tue, 25 Oct 2016 17:02:47 +0200
From: Colin Vidal <colin@...dal.org>
To: AKASHI Takahiro <takahiro.akashi@...aro.org>
Cc: kernel-hardening@...ts.openwall.com, "Reshetova, Elena"
<elena.reshetova@...el.com>, David Windsor <dave@...gbits.org>, Kees Cook
<keescook@...omium.org>, Hans Liljestrand <ishkamiel@...il.com>
Subject: Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC
On Tue, 2016-10-25 at 18:18 +0900, AKASHI Takahiro wrote:
> On Tue, Oct 18, 2016 at 04:59:21PM +0200, Colin Vidal wrote:
> >
> > This adds arm-specific code in order to support HARDENED_ATOMIC
> > feature. When overflow is detected in atomic_t, atomic64_t or
> > atomic_long_t, an exception is raised and call
> > hardened_atomic_overflow.
> >
> > Signed-off-by: Colin Vidal <colin@...dal.org>
> > ---
> > arch/arm/Kconfig | 1 +
> > arch/arm/include/asm/atomic.h | 434 +++++++++++++++++++++++++++++-------------
> > arch/arm/mm/fault.c | 15 ++
> > 3 files changed, 320 insertions(+), 130 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index b5d529f..fcf4a64 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -36,6 +36,7 @@ config ARM
> > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
> > select HAVE_ARCH_HARDENED_USERCOPY
> > + select HAVE_ARCH_HARDENED_ATOMIC
> > select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> > select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
> > select HAVE_ARCH_MMAP_RND_BITS if MMU
> > diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
> > index 66d0e21..fdaee17 100644
> > --- a/arch/arm/include/asm/atomic.h
> > +++ b/arch/arm/include/asm/atomic.h
> > @@ -17,18 +17,52 @@
> > #include <linux/irqflags.h>
> > #include <asm/barrier.h>
> > #include <asm/cmpxchg.h>
> > +#include <linux/bug.h>
> >
> > #define ATOMIC_INIT(i) { (i) }
> >
> > #ifdef __KERNEL__
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +#define HARDENED_ATOMIC_INSN "bkpt 0xf103"
> > +#define _ASM_EXTABLE(from, to) \
> > + ".pushsection __ex_table,\"a\"\n" \
> > + ".align 3\n" \
> > + ".long "#from","#to"\n" \
> > + ".popsection"
> > +#define __OVERFLOW_POST \
> > + "bvc 3f\n" \
> > + "2: "HARDENED_ATOMIC_INSN"\n" \
> > + "3:\n"
> > +#define __OVERFLOW_POST_RETURN \
> > + "bvc 3f\n" \
> > + "mov %0,%1\n" \
> > + "2: "HARDENED_ATOMIC_INSN"\n" \
> > + "3:\n"
> > +#define __OVERFLOW_EXTABLE \
> > + "4:\n" \
> > + _ASM_EXTABLE(2b, 4b)
> > +#else
> > +#define __OVERFLOW_POST
> > +#define __OVERFLOW_POST_RETURN
> > +#define __OVERFLOW_EXTABLE
> > +#endif
> > +
> > /*
> > * On ARM, ordinary assignment (str instruction) doesn't clear the local
> > * strex/ldrex monitor on some implementations. The reason we can use it for
> > * atomic_set() is the clrex or dummy strex done on every exception return.
> > */
> > #define atomic_read(v) READ_ONCE((v)->counter)
> > +static inline int atomic_read_wrap(const atomic_wrap_t *v)
> > +{
> > + return atomic_read(v);
> > +}
> > #define atomic_set(v,i) WRITE_ONCE(((v)->counter), (i))
> > +static inline void atomic_set_wrap(atomic_wrap_t *v, int i)
> > +{
> > + atomic_set(v, i);
> > +}
> >
> > #if __LINUX_ARM_ARCH__ >= 6
> >
> > @@ -38,38 +72,46 @@
> > * to ensure that the update happens.
> > */
> >
> > -#define ATOMIC_OP(op, c_op, asm_op) \
> > -static inline void atomic_##op(int i, atomic_t *v) \
> > +#define __ATOMIC_OP(op, suffix, c_op, asm_op, post_op, extable) \
> > +static inline void atomic_##op##suffix(int i, atomic##suffix##_t *v) \
> > { \
> > unsigned long tmp; \
> > int result; \
> > \
> > prefetchw(&v->counter); \
> > - __asm__ __volatile__("@ atomic_" #op "\n" \
> > + __asm__ __volatile__("@ atomic_" #op #suffix "\n" \
> > "1: ldrex %0, [%3]\n" \
> > " " #asm_op " %0, %0, %4\n" \
> > + post_op \
> > " strex %1, %0, [%3]\n" \
> > " teq %1, #0\n" \
> > -" bne 1b" \
> > +" bne 1b\n" \
> > + extable \
> > : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) \
> > : "r" (&v->counter), "Ir" (i) \
> > : "cc"); \
> > } \
> >
> > -#define ATOMIC_OP_RETURN(op, c_op, asm_op) \
> > -static inline int atomic_##op##_return_relaxed(int i, atomic_t *v) \
> > +#define ATOMIC_OP(op, c_op, asm_op) \
> > + __ATOMIC_OP(op, _wrap, c_op, asm_op, , ) \
> > + __ATOMIC_OP(op, , c_op, asm_op##s, __OVERFLOW_POST, __OVERFLOW_EXTABLE)
> > +
> > +#define __ATOMIC_OP_RETURN(op, suffix, c_op, asm_op, post_op, extable) \
> > +static inline int atomic_##op##_return##suffix##_relaxed(int i, atomic##suffix##_t *v) \
> > { \
> > unsigned long tmp; \
> > int result; \
> > \
> > prefetchw(&v->counter); \
> > \
> > - __asm__ __volatile__("@ atomic_" #op "_return\n" \
> > + __asm__ __volatile__("@ atomic_" #op "_return" #suffix "\n" \
> > "1: ldrex %0, [%3]\n" \
> > " " #asm_op " %0, %0, %4\n" \
> > + post_op \
> > " strex %1, %0, [%3]\n" \
> > " teq %1, #0\n" \
> > -" bne 1b" \
> > +" bne 1b\n" \
> > + extable \
> > : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) \
> > : "r" (&v->counter), "Ir" (i) \
> > : "cc"); \
> > @@ -77,6 +119,11 @@ static inline int atomic_##op##_return_relaxed(int i, atomic_t *v) \
> > return result; \
> > }
> >
> > +#define ATOMIC_OP_RETURN(op, c_op, asm_op) \
> > + __ATOMIC_OP_RETURN(op, _wrap, c_op, asm_op, , ) \
> > + __ATOMIC_OP_RETURN(op, , c_op, asm_op##s, \
> > + __OVERFLOW_POST_RETURN, __OVERFLOW_EXTABLE)
> > +
>
> This definition will create atomic_add_return_wrap_relaxed(),
> but should the name be atomic_add_return_relaxed_wrap()?
>
> (I don't know we need _wrap version of _relaxed functions.
> See Elena's atomic-long.h.)
>
> Thanks,
> -Takahiro AKASHI
Hi Takahiro,
as far I understand, the name should be atomic_add_return_wrap_relaxed
since atomic_add_return_wrap is created by __atomic_op_fence (in order
to put memory barrier around the call to
atomic_add_return_wrap_relaxed) in include/linux/atomic.h.
Am I wrong?
Thanks!
Colin
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.