* [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC @ 2016-10-18 14:59 Colin Vidal 2016-10-18 14:59 ` [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset Colin Vidal ` (3 more replies) 0 siblings, 4 replies; 30+ messages in thread From: Colin Vidal @ 2016-10-18 14:59 UTC (permalink / raw) To: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand Cc: Colin Vidal Hi, This is the first attempt of HARDENED_ATOMIC port to arm arch. About the fault handling I have some questions (perhaps some arm expert are reading?): - As the process that made the overflow is killed, the kernel will not try to go to a fixup address when the exception is raised, right ? Therefore, is still mandatory to add an entry in the __extable section? - In do_PrefetchAbort, I am unsure the code that follow the call to hardened_atomic_overflow is needed: the process will be killed anyways. I take some freedom compared to PaX patch, especially by adding some macro to expand functions in arm/include/asm/atomic.h. The first patch is the modification I have done is generic part to make it work. Otherwise, I've been stuck by ccache. When I modify do_PrefetchAbort in arm/mm/fault.c, ccache does not detect the update (even if the file is recompiled by gcc). Therefore, when I boot the new compiled kernel, the old version of do_PrefechAbort is called. I know do_PrefetchAbort is somehow special, since called by assembly code, but is still strange. Someone has already has this issue? The only way to solve it is to flush the cache... Thanks! Colin Colin Vidal (2): Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. arm: implementation for HARDENED_ATOMIC arch/arm/Kconfig | 1 + arch/arm/include/asm/atomic.h | 434 ++++++++++++++++++++++++++------------ arch/arm/mm/fault.c | 15 ++ include/asm-generic/atomic-long.h | 55 ++--- include/linux/atomic.h | 55 +++++ 5 files changed, 405 insertions(+), 155 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-18 14:59 [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC Colin Vidal @ 2016-10-18 14:59 ` Colin Vidal 2016-10-18 16:04 ` Vaishali Thakkar 2016-10-19 8:21 ` [kernel-hardening] " Reshetova, Elena 2016-10-18 14:59 ` [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC Colin Vidal ` (2 subsequent siblings) 3 siblings, 2 replies; 30+ messages in thread From: Colin Vidal @ 2016-10-18 14:59 UTC (permalink / raw) To: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand Cc: Colin Vidal Signed-off-by: Colin Vidal <colin@cvidal.org> --- include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------ include/linux/atomic.h | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 25 deletions(-) diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h index 790cb00..94d712b 100644 --- a/include/asm-generic/atomic-long.h +++ b/include/asm-generic/atomic-long.h @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t; #endif +#ifndef CONFIG_HARDENED_ATOMIC +#define atomic_read_wrap(v) atomic_read(v) +#define atomic_set_wrap(v, i) atomic_set((v), (i)) +#define atomic_add_wrap(i, v) atomic_add((i), (v)) +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) +#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) +#define atomic_inc_wrap(v) atomic_inc(v) +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) +#ifndef atomic_inc_return_wrap +#define atomic_inc_return_wrap(v) atomic_inc_return(v) +#endif +#ifndef atomic_add_return_wrap +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) +#endif +#define atomic_dec_wrap(v) atomic_dec(v) +#ifndef atomic_xchg_wrap +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) +#endif +#define atomic_long_inc_wrap(v) atomic_long_inc(v) +#define atomic_long_dec_wrap(v) atomic_long_dec(v) +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) +#endif /* CONFIG_HARDENED_ATOMIC */ + #define ATOMIC_LONG_READ_OP(mo, suffix) \ static inline long atomic_long_read##mo##suffix(const atomic_long##suffix##_t *l)\ { \ @@ -104,6 +128,12 @@ ATOMIC_LONG_ADD_SUB_OP(sub, _release,) #define atomic_long_cmpxchg(l, old, new) \ (ATOMIC_LONG_PFX(_cmpxchg)((ATOMIC_LONG_PFX(_t) *)(l), (old), (new))) +#ifdef CONFIG_HARDENED_ATOMIC +#define atomic_long_cmpxchg_wrap(l, old, new) \ + (ATOMIC_LONG_PFX(_cmpxchg_wrap)((ATOMIC_LONG_PFX(_wrap_t) *)(l),\ + (old), (new))) +#endif + #define atomic_long_xchg_relaxed(v, new) \ (ATOMIC_LONG_PFX(_xchg_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (new))) #define atomic_long_xchg_acquire(v, new) \ @@ -291,29 +321,4 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u) #define atomic_long_inc_not_zero(l) \ ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l)) -#ifndef CONFIG_HARDENED_ATOMIC -#define atomic_read_wrap(v) atomic_read(v) -#define atomic_set_wrap(v, i) atomic_set((v), (i)) -#define atomic_add_wrap(i, v) atomic_add((i), (v)) -#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) -#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) -#define atomic_inc_wrap(v) atomic_inc(v) -#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) -#define atomic_inc_return_wrap(v) atomic_inc_return(v) -#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) -#define atomic_dec_wrap(v) atomic_dec(v) -#define atomic_cmpxchg_wrap(v, o, n) atomic_cmpxchg((v), (o), (n)) -#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) -#define atomic_long_read_wrap(v) atomic_long_read(v) -#define atomic_long_set_wrap(v, i) atomic_long_set((v), (i)) -#define atomic_long_add_wrap(i, v) atomic_long_add((i), (v)) -#define atomic_long_sub_wrap(i, v) atomic_long_sub((i), (v)) -#define atomic_long_inc_wrap(v) atomic_long_inc(v) -#define atomic_long_add_return_wrap(i, v) atomic_long_add_return((i), (v)) -#define atomic_long_inc_return_wrap(v) atomic_long_inc_return(v) -#define atomic_long_sub_and_test_wrap(i, v) atomic_long_sub_and_test((i), (v)) -#define atomic_long_dec_wrap(v) atomic_long_dec(v) -#define atomic_long_xchg_wrap(v, i) atomic_long_xchg((v), (i)) -#endif /* CONFIG_HARDENED_ATOMIC */ - #endif /* _ASM_GENERIC_ATOMIC_LONG_H */ diff --git a/include/linux/atomic.h b/include/linux/atomic.h index b5817c8..be16ea1 100644 --- a/include/linux/atomic.h +++ b/include/linux/atomic.h @@ -89,6 +89,11 @@ #define atomic_add_return(...) \ __atomic_op_fence(atomic_add_return, __VA_ARGS__) #endif + +#ifndef atomic_add_return_wrap_relaxed +#define atomic_add_return_wrap(...) \ + __atomic_op_fence(atomic_add_return_wrap, __VA_ARGS__) +#endif #endif /* atomic_add_return_relaxed */ /* atomic_inc_return_relaxed */ @@ -113,6 +118,11 @@ #define atomic_inc_return(...) \ __atomic_op_fence(atomic_inc_return, __VA_ARGS__) #endif + +#ifndef atomic_inc_return_wrap +#define atomic_inc_return_wrap(...) \ + __atomic_op_fence(atomic_inc_return_wrap, __VA_ARGS__) +#endif #endif /* atomic_inc_return_relaxed */ /* atomic_sub_return_relaxed */ @@ -137,6 +147,11 @@ #define atomic_sub_return(...) \ __atomic_op_fence(atomic_sub_return, __VA_ARGS__) #endif + +#ifndef atomic_sub_return_wrap_relaxed +#define atomic_sub_return_wrap(...) \ + __atomic_op_fence(atomic_sub_return_wrap, __VA_ARGS__) +#endif #endif /* atomic_sub_return_relaxed */ /* atomic_dec_return_relaxed */ @@ -161,6 +176,11 @@ #define atomic_dec_return(...) \ __atomic_op_fence(atomic_dec_return, __VA_ARGS__) #endif + +#ifndef atomic_dec_return_wrap +#define atomic_dec_return_wrap(...) \ + __atomic_op_fence(atomic_dec_return, __VA_ARGS__) +#endif #endif /* atomic_dec_return_relaxed */ @@ -421,6 +441,11 @@ #define atomic_cmpxchg(...) \ __atomic_op_fence(atomic_cmpxchg, __VA_ARGS__) #endif + +#ifndef atomic_cmpxchg_wrap +#define atomic_cmpxchg_wrap(...) \ + __atomic_op_fence(atomic_cmpxchg_wrap, __VA_ARGS__) +#endif #endif /* atomic_cmpxchg_relaxed */ /* cmpxchg_relaxed */ @@ -675,6 +700,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) #define atomic64_add_return(...) \ __atomic_op_fence(atomic64_add_return, __VA_ARGS__) #endif + +#ifndef atomic64_add_return_wrap +#define atomic64_add_return_wrap(...) \ + __atomic_op_fence(atomic64_add_return_wrap, __VA_ARGS__) +#endif #endif /* atomic64_add_return_relaxed */ /* atomic64_inc_return_relaxed */ @@ -699,6 +729,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) #define atomic64_inc_return(...) \ __atomic_op_fence(atomic64_inc_return, __VA_ARGS__) #endif + +#ifndef atomic64_inc_return_wrap +#define atomic64_inc_return_wrap(...) \ + __atomic_op_fence(atomic64_inc_return_wrap, __VA_ARGS__) +#endif #endif /* atomic64_inc_return_relaxed */ @@ -724,6 +759,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) #define atomic64_sub_return(...) \ __atomic_op_fence(atomic64_sub_return, __VA_ARGS__) #endif + +#ifndef atomic64_sub_return_wrap +#define atomic64_sub_return_wrap(...) \ + __atomic_op_fence(atomic64_sub_return_wrap, __VA_ARGS__) +#endif #endif /* atomic64_sub_return_relaxed */ /* atomic64_dec_return_relaxed */ @@ -748,6 +788,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) #define atomic64_dec_return(...) \ __atomic_op_fence(atomic64_dec_return, __VA_ARGS__) #endif + +#ifndef atomic64_dec_return_wrap +#define atomic64_dec_return_wrap(...) \ + __atomic_op_fence(atomic64_dec_return_wrap, __VA_ARGS__) +#endif #endif /* atomic64_dec_return_relaxed */ @@ -984,6 +1029,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) #define atomic64_xchg(...) \ __atomic_op_fence(atomic64_xchg, __VA_ARGS__) #endif + +#ifndef atomic64_xchg_wrap +#define atomic64_xchg_wrap(...) \ + __atomic_op_fence(atomic64_xchg_wrap, __VA_ARGS__) +#endif #endif /* atomic64_xchg_relaxed */ /* atomic64_cmpxchg_relaxed */ @@ -1008,6 +1058,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) #define atomic64_cmpxchg(...) \ __atomic_op_fence(atomic64_cmpxchg, __VA_ARGS__) #endif + +#ifndef atomic64_cmpxchg_wrap +#define atomic64_cmpxchg_wrap(...) \ + __atomic_op_fence(atomic64_cmpxchg_wrap, __VA_ARGS__) +#endif #endif /* atomic64_cmpxchg_relaxed */ #ifndef atomic64_andnot -- 2.7.4 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-18 14:59 ` [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset Colin Vidal @ 2016-10-18 16:04 ` Vaishali Thakkar 2016-10-19 8:48 ` Colin Vidal 2016-10-19 8:21 ` [kernel-hardening] " Reshetova, Elena 1 sibling, 1 reply; 30+ messages in thread From: Vaishali Thakkar @ 2016-10-18 16:04 UTC (permalink / raw) To: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand Cc: Colin Vidal On Tuesday 18 October 2016 08:29 PM, Colin Vidal wrote: > Signed-off-by: Colin Vidal <colin@cvidal.org> Hi, While I can't comment on technical things because of my limited arm specific knowledge [although these are simple changes, I would let others comment on this], I think subject is too long according to the kernel's patch submission guidelines. Also, I know these are simple mechanic changes. But I still think that having a commit log can help here. May be you can have something similar to Elena's x86 patches. Your other patch in the series looks good to me. :) Thanks. > --- > include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------ > include/linux/atomic.h | 55 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 85 insertions(+), 25 deletions(-) > > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h > index 790cb00..94d712b 100644 > --- a/include/asm-generic/atomic-long.h > +++ b/include/asm-generic/atomic-long.h > @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t; > > #endif > > +#ifndef CONFIG_HARDENED_ATOMIC > +#define atomic_read_wrap(v) atomic_read(v) > +#define atomic_set_wrap(v, i) atomic_set((v), (i)) > +#define atomic_add_wrap(i, v) atomic_add((i), (v)) > +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) > +#define atomic_inc_wrap(v) atomic_inc(v) > +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) > +#ifndef atomic_inc_return_wrap > +#define atomic_inc_return_wrap(v) atomic_inc_return(v) > +#endif > +#ifndef atomic_add_return_wrap > +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) > +#endif > +#define atomic_dec_wrap(v) atomic_dec(v) > +#ifndef atomic_xchg_wrap > +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) > +#endif > +#define atomic_long_inc_wrap(v) atomic_long_inc(v) > +#define atomic_long_dec_wrap(v) atomic_long_dec(v) > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) > +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) > +#endif /* CONFIG_HARDENED_ATOMIC */ > + > #define ATOMIC_LONG_READ_OP(mo, suffix) \ > static inline long atomic_long_read##mo##suffix(const atomic_long##suffix##_t *l)\ > { \ > @@ -104,6 +128,12 @@ ATOMIC_LONG_ADD_SUB_OP(sub, _release,) > #define atomic_long_cmpxchg(l, old, new) \ > (ATOMIC_LONG_PFX(_cmpxchg)((ATOMIC_LONG_PFX(_t) *)(l), (old), (new))) > > +#ifdef CONFIG_HARDENED_ATOMIC > +#define atomic_long_cmpxchg_wrap(l, old, new) \ > + (ATOMIC_LONG_PFX(_cmpxchg_wrap)((ATOMIC_LONG_PFX(_wrap_t) *)(l),\ > + (old), (new))) > +#endif > + > #define atomic_long_xchg_relaxed(v, new) \ > (ATOMIC_LONG_PFX(_xchg_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (new))) > #define atomic_long_xchg_acquire(v, new) \ > @@ -291,29 +321,4 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u) > #define atomic_long_inc_not_zero(l) \ > ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l)) > > -#ifndef CONFIG_HARDENED_ATOMIC > -#define atomic_read_wrap(v) atomic_read(v) > -#define atomic_set_wrap(v, i) atomic_set((v), (i)) > -#define atomic_add_wrap(i, v) atomic_add((i), (v)) > -#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) > -#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) > -#define atomic_inc_wrap(v) atomic_inc(v) > -#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) > -#define atomic_inc_return_wrap(v) atomic_inc_return(v) > -#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) > -#define atomic_dec_wrap(v) atomic_dec(v) > -#define atomic_cmpxchg_wrap(v, o, n) atomic_cmpxchg((v), (o), (n)) > -#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) > -#define atomic_long_read_wrap(v) atomic_long_read(v) > -#define atomic_long_set_wrap(v, i) atomic_long_set((v), (i)) > -#define atomic_long_add_wrap(i, v) atomic_long_add((i), (v)) > -#define atomic_long_sub_wrap(i, v) atomic_long_sub((i), (v)) > -#define atomic_long_inc_wrap(v) atomic_long_inc(v) > -#define atomic_long_add_return_wrap(i, v) atomic_long_add_return((i), (v)) > -#define atomic_long_inc_return_wrap(v) atomic_long_inc_return(v) > -#define atomic_long_sub_and_test_wrap(i, v) atomic_long_sub_and_test((i), (v)) > -#define atomic_long_dec_wrap(v) atomic_long_dec(v) > -#define atomic_long_xchg_wrap(v, i) atomic_long_xchg((v), (i)) > -#endif /* CONFIG_HARDENED_ATOMIC */ > - > #endif /* _ASM_GENERIC_ATOMIC_LONG_H */ > diff --git a/include/linux/atomic.h b/include/linux/atomic.h > index b5817c8..be16ea1 100644 > --- a/include/linux/atomic.h > +++ b/include/linux/atomic.h > @@ -89,6 +89,11 @@ > #define atomic_add_return(...) \ > __atomic_op_fence(atomic_add_return, __VA_ARGS__) > #endif > + > +#ifndef atomic_add_return_wrap_relaxed > +#define atomic_add_return_wrap(...) \ > + __atomic_op_fence(atomic_add_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic_add_return_relaxed */ > > /* atomic_inc_return_relaxed */ > @@ -113,6 +118,11 @@ > #define atomic_inc_return(...) \ > __atomic_op_fence(atomic_inc_return, __VA_ARGS__) > #endif > + > +#ifndef atomic_inc_return_wrap > +#define atomic_inc_return_wrap(...) \ > + __atomic_op_fence(atomic_inc_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic_inc_return_relaxed */ > > /* atomic_sub_return_relaxed */ > @@ -137,6 +147,11 @@ > #define atomic_sub_return(...) \ > __atomic_op_fence(atomic_sub_return, __VA_ARGS__) > #endif > + > +#ifndef atomic_sub_return_wrap_relaxed > +#define atomic_sub_return_wrap(...) \ > + __atomic_op_fence(atomic_sub_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic_sub_return_relaxed */ > > /* atomic_dec_return_relaxed */ > @@ -161,6 +176,11 @@ > #define atomic_dec_return(...) \ > __atomic_op_fence(atomic_dec_return, __VA_ARGS__) > #endif > + > +#ifndef atomic_dec_return_wrap > +#define atomic_dec_return_wrap(...) \ > + __atomic_op_fence(atomic_dec_return, __VA_ARGS__) > +#endif > #endif /* atomic_dec_return_relaxed */ > > > @@ -421,6 +441,11 @@ > #define atomic_cmpxchg(...) \ > __atomic_op_fence(atomic_cmpxchg, __VA_ARGS__) > #endif > + > +#ifndef atomic_cmpxchg_wrap > +#define atomic_cmpxchg_wrap(...) \ > + __atomic_op_fence(atomic_cmpxchg_wrap, __VA_ARGS__) > +#endif > #endif /* atomic_cmpxchg_relaxed */ > > /* cmpxchg_relaxed */ > @@ -675,6 +700,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > #define atomic64_add_return(...) \ > __atomic_op_fence(atomic64_add_return, __VA_ARGS__) > #endif > + > +#ifndef atomic64_add_return_wrap > +#define atomic64_add_return_wrap(...) \ > + __atomic_op_fence(atomic64_add_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic64_add_return_relaxed */ > > /* atomic64_inc_return_relaxed */ > @@ -699,6 +729,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > #define atomic64_inc_return(...) \ > __atomic_op_fence(atomic64_inc_return, __VA_ARGS__) > #endif > + > +#ifndef atomic64_inc_return_wrap > +#define atomic64_inc_return_wrap(...) \ > + __atomic_op_fence(atomic64_inc_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic64_inc_return_relaxed */ > > > @@ -724,6 +759,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > #define atomic64_sub_return(...) \ > __atomic_op_fence(atomic64_sub_return, __VA_ARGS__) > #endif > + > +#ifndef atomic64_sub_return_wrap > +#define atomic64_sub_return_wrap(...) \ > + __atomic_op_fence(atomic64_sub_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic64_sub_return_relaxed */ > > /* atomic64_dec_return_relaxed */ > @@ -748,6 +788,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > #define atomic64_dec_return(...) \ > __atomic_op_fence(atomic64_dec_return, __VA_ARGS__) > #endif > + > +#ifndef atomic64_dec_return_wrap > +#define atomic64_dec_return_wrap(...) \ > + __atomic_op_fence(atomic64_dec_return_wrap, __VA_ARGS__) > +#endif > #endif /* atomic64_dec_return_relaxed */ > > > @@ -984,6 +1029,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > #define atomic64_xchg(...) \ > __atomic_op_fence(atomic64_xchg, __VA_ARGS__) > #endif > + > +#ifndef atomic64_xchg_wrap > +#define atomic64_xchg_wrap(...) \ > + __atomic_op_fence(atomic64_xchg_wrap, __VA_ARGS__) > +#endif > #endif /* atomic64_xchg_relaxed */ > > /* atomic64_cmpxchg_relaxed */ > @@ -1008,6 +1058,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > #define atomic64_cmpxchg(...) \ > __atomic_op_fence(atomic64_cmpxchg, __VA_ARGS__) > #endif > + > +#ifndef atomic64_cmpxchg_wrap > +#define atomic64_cmpxchg_wrap(...) \ > + __atomic_op_fence(atomic64_cmpxchg_wrap, __VA_ARGS__) > +#endif > #endif /* atomic64_cmpxchg_relaxed */ > > #ifndef atomic64_andnot > -- Vaishali ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-18 16:04 ` Vaishali Thakkar @ 2016-10-19 8:48 ` Colin Vidal 0 siblings, 0 replies; 30+ messages in thread From: Colin Vidal @ 2016-10-19 8:48 UTC (permalink / raw) To: Vaishali Thakkar, kernel-hardening Hi, On Tue, 2016-10-18 at 21:34 +0530, Vaishali Thakkar wrote: > > On Tuesday 18 October 2016 08:29 PM, Colin Vidal wrote: > > > > Signed-off-by: Colin Vidal <colin@cvidal.org> > > Hi, > > While I can't comment on technical things because of my limited arm > specific knowledge [although these are simple changes, I would let > others comment on this], I think subject is too long according to the > kernel's patch submission guidelines. > > Also, I know these are simple mechanic changes. But I still think > that having a commit log can help here. May be you can have something > similar to Elena's x86 patches. > > Your other patch in the series looks good to me. :) > Thanks about those observations, I fix that for next RFC! Colin > Thanks. > > > > > > --- > > include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------ > > include/linux/atomic.h | 55 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 85 insertions(+), 25 deletions(-) > > > > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h > > index 790cb00..94d712b 100644 > > --- a/include/asm-generic/atomic-long.h > > +++ b/include/asm-generic/atomic-long.h > > @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t; > > > > #endif > > > > +#ifndef CONFIG_HARDENED_ATOMIC > > +#define atomic_read_wrap(v) atomic_read(v) > > +#define atomic_set_wrap(v, i) atomic_set((v), (i)) > > +#define atomic_add_wrap(i, v) atomic_add((i), (v)) > > +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) > > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) > > +#define atomic_inc_wrap(v) atomic_inc(v) > > +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) > > +#ifndef atomic_inc_return_wrap > > +#define atomic_inc_return_wrap(v) atomic_inc_return(v) > > +#endif > > +#ifndef atomic_add_return_wrap > > +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) > > +#endif > > +#define atomic_dec_wrap(v) atomic_dec(v) > > +#ifndef atomic_xchg_wrap > > +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) > > +#endif > > +#define atomic_long_inc_wrap(v) atomic_long_inc(v) > > +#define atomic_long_dec_wrap(v) atomic_long_dec(v) > > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) > > +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) > > +#endif /* CONFIG_HARDENED_ATOMIC */ > > + > > #define ATOMIC_LONG_READ_OP(mo, suffix) \ > > static inline long atomic_long_read##mo##suffix(const atomic_long##suffix##_t *l)\ > > { \ > > @@ -104,6 +128,12 @@ ATOMIC_LONG_ADD_SUB_OP(sub, _release,) > > #define atomic_long_cmpxchg(l, old, new) \ > > (ATOMIC_LONG_PFX(_cmpxchg)((ATOMIC_LONG_PFX(_t) *)(l), (old), (new))) > > > > +#ifdef CONFIG_HARDENED_ATOMIC > > +#define atomic_long_cmpxchg_wrap(l, old, new) \ > > + (ATOMIC_LONG_PFX(_cmpxchg_wrap)((ATOMIC_LONG_PFX(_wrap_t) *)(l),\ > > + (old), (new))) > > +#endif > > + > > #define atomic_long_xchg_relaxed(v, new) \ > > (ATOMIC_LONG_PFX(_xchg_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (new))) > > #define atomic_long_xchg_acquire(v, new) \ > > @@ -291,29 +321,4 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u) > > #define atomic_long_inc_not_zero(l) \ > > ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l)) > > > > -#ifndef CONFIG_HARDENED_ATOMIC > > -#define atomic_read_wrap(v) atomic_read(v) > > -#define atomic_set_wrap(v, i) atomic_set((v), (i)) > > -#define atomic_add_wrap(i, v) atomic_add((i), (v)) > > -#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) > > -#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) > > -#define atomic_inc_wrap(v) atomic_inc(v) > > -#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) > > -#define atomic_inc_return_wrap(v) atomic_inc_return(v) > > -#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) > > -#define atomic_dec_wrap(v) atomic_dec(v) > > -#define atomic_cmpxchg_wrap(v, o, n) atomic_cmpxchg((v), (o), (n)) > > -#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) > > -#define atomic_long_read_wrap(v) atomic_long_read(v) > > -#define atomic_long_set_wrap(v, i) atomic_long_set((v), (i)) > > -#define atomic_long_add_wrap(i, v) atomic_long_add((i), (v)) > > -#define atomic_long_sub_wrap(i, v) atomic_long_sub((i), (v)) > > -#define atomic_long_inc_wrap(v) atomic_long_inc(v) > > -#define atomic_long_add_return_wrap(i, v) atomic_long_add_return((i), (v)) > > -#define atomic_long_inc_return_wrap(v) atomic_long_inc_return(v) > > -#define atomic_long_sub_and_test_wrap(i, v) atomic_long_sub_and_test((i), (v)) > > -#define atomic_long_dec_wrap(v) atomic_long_dec(v) > > -#define atomic_long_xchg_wrap(v, i) atomic_long_xchg((v), (i)) > > -#endif /* CONFIG_HARDENED_ATOMIC */ > > - > > #endif /* _ASM_GENERIC_ATOMIC_LONG_H */ > > diff --git a/include/linux/atomic.h b/include/linux/atomic.h > > index b5817c8..be16ea1 100644 > > --- a/include/linux/atomic.h > > +++ b/include/linux/atomic.h > > @@ -89,6 +89,11 @@ > > #define atomic_add_return(...) \ > > __atomic_op_fence(atomic_add_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic_add_return_wrap_relaxed > > +#define atomic_add_return_wrap(...) \ > > + __atomic_op_fence(atomic_add_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic_add_return_relaxed */ > > > > /* atomic_inc_return_relaxed */ > > @@ -113,6 +118,11 @@ > > #define atomic_inc_return(...) \ > > __atomic_op_fence(atomic_inc_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic_inc_return_wrap > > +#define atomic_inc_return_wrap(...) \ > > + __atomic_op_fence(atomic_inc_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic_inc_return_relaxed */ > > > > /* atomic_sub_return_relaxed */ > > @@ -137,6 +147,11 @@ > > #define atomic_sub_return(...) \ > > __atomic_op_fence(atomic_sub_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic_sub_return_wrap_relaxed > > +#define atomic_sub_return_wrap(...) \ > > + __atomic_op_fence(atomic_sub_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic_sub_return_relaxed */ > > > > /* atomic_dec_return_relaxed */ > > @@ -161,6 +176,11 @@ > > #define atomic_dec_return(...) \ > > __atomic_op_fence(atomic_dec_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic_dec_return_wrap > > +#define atomic_dec_return_wrap(...) \ > > + __atomic_op_fence(atomic_dec_return, __VA_ARGS__) > > +#endif > > #endif /* atomic_dec_return_relaxed */ > > > > > > @@ -421,6 +441,11 @@ > > #define atomic_cmpxchg(...) \ > > __atomic_op_fence(atomic_cmpxchg, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic_cmpxchg_wrap > > +#define atomic_cmpxchg_wrap(...) \ > > + __atomic_op_fence(atomic_cmpxchg_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic_cmpxchg_relaxed */ > > > > /* cmpxchg_relaxed */ > > @@ -675,6 +700,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > > #define atomic64_add_return(...) \ > > __atomic_op_fence(atomic64_add_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic64_add_return_wrap > > +#define atomic64_add_return_wrap(...) \ > > + __atomic_op_fence(atomic64_add_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic64_add_return_relaxed */ > > > > /* atomic64_inc_return_relaxed */ > > @@ -699,6 +729,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > > #define atomic64_inc_return(...) \ > > __atomic_op_fence(atomic64_inc_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic64_inc_return_wrap > > +#define atomic64_inc_return_wrap(...) \ > > + __atomic_op_fence(atomic64_inc_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic64_inc_return_relaxed */ > > > > > > @@ -724,6 +759,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > > #define atomic64_sub_return(...) \ > > __atomic_op_fence(atomic64_sub_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic64_sub_return_wrap > > +#define atomic64_sub_return_wrap(...) \ > > + __atomic_op_fence(atomic64_sub_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic64_sub_return_relaxed */ > > > > /* atomic64_dec_return_relaxed */ > > @@ -748,6 +788,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > > #define atomic64_dec_return(...) \ > > __atomic_op_fence(atomic64_dec_return, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic64_dec_return_wrap > > +#define atomic64_dec_return_wrap(...) \ > > + __atomic_op_fence(atomic64_dec_return_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic64_dec_return_relaxed */ > > > > > > @@ -984,6 +1029,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > > #define atomic64_xchg(...) \ > > __atomic_op_fence(atomic64_xchg, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic64_xchg_wrap > > +#define atomic64_xchg_wrap(...) \ > > + __atomic_op_fence(atomic64_xchg_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic64_xchg_relaxed */ > > > > /* atomic64_cmpxchg_relaxed */ > > @@ -1008,6 +1058,11 @@ static inline int atomic_dec_if_positive(atomic_t *v) > > #define atomic64_cmpxchg(...) \ > > __atomic_op_fence(atomic64_cmpxchg, __VA_ARGS__) > > #endif > > + > > +#ifndef atomic64_cmpxchg_wrap > > +#define atomic64_cmpxchg_wrap(...) \ > > + __atomic_op_fence(atomic64_cmpxchg_wrap, __VA_ARGS__) > > +#endif > > #endif /* atomic64_cmpxchg_relaxed */ > > > > #ifndef atomic64_andnot > > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] RE: [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-18 14:59 ` [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset Colin Vidal 2016-10-18 16:04 ` Vaishali Thakkar @ 2016-10-19 8:21 ` Reshetova, Elena 2016-10-19 8:31 ` Greg KH 1 sibling, 1 reply; 30+ messages in thread From: Reshetova, Elena @ 2016-10-19 8:21 UTC (permalink / raw) To: Colin Vidal, kernel-hardening, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand Signed-off-by: Colin Vidal <colin@cvidal.org> --- include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------ include/linux/atomic.h | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 25 deletions(-) diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h index 790cb00..94d712b 100644 --- a/include/asm-generic/atomic-long.h +++ b/include/asm-generic/atomic-long.h @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t; #endif +#ifndef CONFIG_HARDENED_ATOMIC +#define atomic_read_wrap(v) atomic_read(v) #define atomic_set_wrap(v, +i) atomic_set((v), (i)) #define atomic_add_wrap(i, v) atomic_add((i), +(v)) #define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), +(i), (j)) #define atomic_sub_wrap(i, v) atomic_sub((i), (v)) #define +atomic_inc_wrap(v) atomic_inc(v) #define atomic_inc_and_test_wrap(v) +atomic_inc_and_test(v) #ifndef atomic_inc_return_wrap #define +atomic_inc_return_wrap(v) atomic_inc_return(v) #endif #ifndef +atomic_add_return_wrap #define atomic_add_return_wrap(i, v) +atomic_add_return((i), (v)) #endif #define atomic_dec_wrap(v) +atomic_dec(v) #ifndef atomic_xchg_wrap #define atomic_xchg_wrap(v, i) +atomic_xchg((v), (i)) #endif #define atomic_long_inc_wrap(v) +atomic_long_inc(v) #define atomic_long_dec_wrap(v) atomic_long_dec(v) +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) #define +atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) #endif +/* CONFIG_HARDENED_ATOMIC */ + #define ATOMIC_LONG_READ_OP(mo, suffix) \ static inline long atomic_long_read##mo##suffix(const atomic_long##suffix##_t *l)\ { \ @@ -104,6 +128,12 @@ ATOMIC_LONG_ADD_SUB_OP(sub, _release,) #define atomic_long_cmpxchg(l, old, new) \ (ATOMIC_LONG_PFX(_cmpxchg)((ATOMIC_LONG_PFX(_t) *)(l), (old), (new))) +#ifdef CONFIG_HARDENED_ATOMIC +#define atomic_long_cmpxchg_wrap(l, old, new) \ + (ATOMIC_LONG_PFX(_cmpxchg_wrap)((ATOMIC_LONG_PFX(_wrap_t) *)(l),\ + (old), (new))) +#endif + #define atomic_long_xchg_relaxed(v, new) \ (ATOMIC_LONG_PFX(_xchg_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (new))) #define atomic_long_xchg_acquire(v, new) \ @@ -291,29 +321,4 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u) #define atomic_long_inc_not_zero(l) \ ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l)) -#ifndef CONFIG_HARDENED_ATOMIC -#define atomic_read_wrap(v) atomic_read(v) -#define atomic_set_wrap(v, i) atomic_set((v), (i)) -#define atomic_add_wrap(i, v) atomic_add((i), (v)) -#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) -#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) -#define atomic_inc_wrap(v) atomic_inc(v) -#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) -#define atomic_inc_return_wrap(v) atomic_inc_return(v) -#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) -#define atomic_dec_wrap(v) atomic_dec(v) -#define atomic_cmpxchg_wrap(v, o, n) atomic_cmpxchg((v), (o), (n)) -#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) -#define atomic_long_read_wrap(v) atomic_long_read(v) -#define atomic_long_set_wrap(v, i) atomic_long_set((v), (i)) -#define atomic_long_add_wrap(i, v) atomic_long_add((i), (v)) -#define atomic_long_sub_wrap(i, v) atomic_long_sub((i), (v)) -#define atomic_long_inc_wrap(v) atomic_long_inc(v) -#define atomic_long_add_return_wrap(i, v) atomic_long_add_return((i), (v)) -#define atomic_long_inc_return_wrap(v) atomic_long_inc_return(v) -#define atomic_long_sub_and_test_wrap(i, v) atomic_long_sub_and_test((i), (v)) -#define atomic_long_dec_wrap(v) atomic_long_dec(v) -#define atomic_long_xchg_wrap(v, i) atomic_long_xchg((v), (i)) -#endif /* CONFIG_HARDENED_ATOMIC */ - #endif /* _ASM_GENERIC_ATOMIC_LONG_H */ diff --git a/include/linux/atomic.h b/include/linux/atomic.h index b5817c8..be16ea1 100644 --- a/include/linux/atomic.h +++ b/include/linux/atomic.h @@ -89,6 +89,11 @@ #define atomic_add_return(...) \ __atomic_op_fence(atomic_add_return, __VA_ARGS__) #endif + +#ifndef atomic_add_return_wrap_relaxed ER: I guess this is a typo, should be ifndef atomic_add_return_wrap +#define atomic_add_return_wrap(...) \ + __atomic_op_fence(atomic_add_return_wrap, __VA_ARGS__) #endif #endif /* atomic_add_return_relaxed */ /* atomic_inc_return_relaxed */ @@ -113,6 +118,11 @@ #define atomic_inc_return(...) \ __atomic_op_fence(atomic_inc_return, __VA_ARGS__) #endif + +#ifndef atomic_inc_return_wrap +#define atomic_inc_return_wrap(...) \ + __atomic_op_fence(atomic_inc_return_wrap, __VA_ARGS__) #endif #endif /* atomic_inc_return_relaxed */ /* atomic_sub_return_relaxed */ @@ -137,6 +147,11 @@ #define atomic_sub_return(...) \ __atomic_op_fence(atomic_sub_return, __VA_ARGS__) #endif + +#ifndef atomic_sub_return_wrap_relaxed ER: Same as above +#define atomic_sub_return_wrap(...) \ + __atomic_op_fence(atomic_sub_return_wrap, __VA_ARGS__) #endif #endif /* atomic_sub_return_relaxed */ /* atomic_dec_return_relaxed */ @@ -161,6 +176,11 @@ #define atomic_dec_return(...) \ __atomic_op_fence(atomic_dec_return, __VA_ARGS__) #endif + +#ifndef atomic_dec_return_wrap +#define atomic_dec_return_wrap(...) \ + __atomic_op_fence(atomic_dec_return, __VA_ARGS__) #endif ER: Another typo? atomic_dec_return_wrap? Actually we will hopefully send next rfc today-tomorrow and we plan to include the above and other general declarations, so hopefully you don't need to carry this part with your rfc anymore! Best Regards, Elena. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] RE: [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-19 8:21 ` [kernel-hardening] " Reshetova, Elena @ 2016-10-19 8:31 ` Greg KH 2016-10-19 8:58 ` Colin Vidal 0 siblings, 1 reply; 30+ messages in thread From: Greg KH @ 2016-10-19 8:31 UTC (permalink / raw) To: kernel-hardening Cc: Colin Vidal, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand On Wed, Oct 19, 2016 at 08:21:06AM +0000, Reshetova, Elena wrote: > Signed-off-by: Colin Vidal <colin@cvidal.org> No changelog at all? > --- > include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------ > include/linux/atomic.h | 55 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 85 insertions(+), 25 deletions(-) > > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h > index 790cb00..94d712b 100644 > --- a/include/asm-generic/atomic-long.h > +++ b/include/asm-generic/atomic-long.h > @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t; > > #endif > > +#ifndef CONFIG_HARDENED_ATOMIC > +#define atomic_read_wrap(v) atomic_read(v) #define atomic_set_wrap(v, > +i) atomic_set((v), (i)) #define atomic_add_wrap(i, v) atomic_add((i), > +(v)) #define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), > +(i), (j)) #define atomic_sub_wrap(i, v) atomic_sub((i), (v)) #define > +atomic_inc_wrap(v) atomic_inc(v) #define atomic_inc_and_test_wrap(v) > +atomic_inc_and_test(v) #ifndef atomic_inc_return_wrap #define > +atomic_inc_return_wrap(v) atomic_inc_return(v) #endif #ifndef > +atomic_add_return_wrap #define atomic_add_return_wrap(i, v) > +atomic_add_return((i), (v)) #endif #define atomic_dec_wrap(v) > +atomic_dec(v) #ifndef atomic_xchg_wrap #define atomic_xchg_wrap(v, i) > +atomic_xchg((v), (i)) #endif #define atomic_long_inc_wrap(v) > +atomic_long_inc(v) #define atomic_long_dec_wrap(v) atomic_long_dec(v) > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) #define > +atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) #endif > +/* CONFIG_HARDENED_ATOMIC */ That doesn't look correct to me at all, does it to you? thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] RE: [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-19 8:31 ` Greg KH @ 2016-10-19 8:58 ` Colin Vidal 2016-10-19 9:16 ` Greg KH 0 siblings, 1 reply; 30+ messages in thread From: Colin Vidal @ 2016-10-19 8:58 UTC (permalink / raw) To: kernel-hardening; +Cc: Greg KH Hi Greg, On Wed, 2016-10-19 at 10:31 +0200, Greg KH wrote: > On Wed, Oct 19, 2016 at 08:21:06AM +0000, Reshetova, Elena wrote: > > > > Signed-off-by: Colin Vidal <colin@cvidal.org> > > No changelog at all? My mistake, I fix that for the next review (some informations was actually in the cover-letter). > > > > --- > > include/asm-generic/atomic-long.h | 55 +++++++++++++++++++++------------------ > > include/linux/atomic.h | 55 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 85 insertions(+), 25 deletions(-) > > > > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h > > index 790cb00..94d712b 100644 > > --- a/include/asm-generic/atomic-long.h > > +++ b/include/asm-generic/atomic-long.h > > @@ -46,6 +46,30 @@ typedef atomic_t atomic_long_wrap_t; > > > > #endif > > > > +#ifndef CONFIG_HARDENED_ATOMIC > > +#define atomic_read_wrap(v) atomic_read(v) > > +#define atomic_set_wrap(v, i) atomic_set((v), (i)) > > +#define atomic_add_wrap(i, v) atomic_add((i), (v)) > > +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) > > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) > > +#define atomic_inc_wrap(v) atomic_inc(v) > > +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) > > +#ifndef atomic_inc_return_wrap > > +#define atomic_inc_return_wrap(v) atomic_inc_return(v) > > +#endif > > +#ifndef atomic_add_return_wrap > > +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) > > +#endif > > +#define atomic_dec_wrap(v) atomic_dec(v) > > +#ifndef atomic_xchg_wrap > > +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) > > +#endif > > +#define atomic_long_inc_wrap(v) atomic_long_inc(v) > > +#define atomic_long_dec_wrap(v) atomic_long_dec(v) > > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) > > +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) > > +#endif /* CONFIG_HARDENED_ATOMIC */ > > That doesn't look correct to me at all, does it to you? That patch will be removed in next version since it will be not needed anymore (Elena's RFC will include those changes). However, I wonder what is obviously wrong here? (in order to avoid it in futures patches). Complex nested #ifdef/#ifndef? Thanks, Colin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] RE: [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset. 2016-10-19 8:58 ` Colin Vidal @ 2016-10-19 9:16 ` Greg KH 0 siblings, 0 replies; 30+ messages in thread From: Greg KH @ 2016-10-19 9:16 UTC (permalink / raw) To: Colin Vidal; +Cc: kernel-hardening On Wed, Oct 19, 2016 at 10:58:09AM +0200, Colin Vidal wrote: > > > +#ifndef CONFIG_HARDENED_ATOMIC > > > +#define atomic_read_wrap(v) atomic_read(v) > > > +#define atomic_set_wrap(v, i) atomic_set((v), (i)) > > > +#define atomic_add_wrap(i, v) atomic_add((i), (v)) > > > +#define atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) > > > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) > > > +#define atomic_inc_wrap(v) atomic_inc(v) > > > +#define atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) > > > +#ifndef atomic_inc_return_wrap > > > +#define atomic_inc_return_wrap(v) atomic_inc_return(v) > > > +#endif > > > +#ifndef atomic_add_return_wrap > > > +#define atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) > > > +#endif > > > +#define atomic_dec_wrap(v) atomic_dec(v) > > > +#ifndef atomic_xchg_wrap > > > +#define atomic_xchg_wrap(v, i) atomic_xchg((v), (i)) > > > +#endif > > > +#define atomic_long_inc_wrap(v) atomic_long_inc(v) > > > +#define atomic_long_dec_wrap(v) atomic_long_dec(v) > > > +#define atomic_long_xchg_wrap(v, n) atomic_long_xchg(v, n) > > > +#define atomic_long_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg(l, o, n) > > > +#endif /* CONFIG_HARDENED_ATOMIC */ > > > > That doesn't look correct to me at all, does it to you? > > That patch will be removed in next version since it will be not needed > anymore (Elena's RFC will include those changes). However, I wonder > what is obviously wrong here? (in order to avoid it in futures > patches). Complex nested #ifdef/#ifndef? Odd, the version I had had all of the line-ends gone, and it was wrapped horribly, but this email shows it correctly. email is fun... greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-18 14:59 [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC Colin Vidal 2016-10-18 14:59 ` [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset Colin Vidal @ 2016-10-18 14:59 ` Colin Vidal 2016-10-18 21:29 ` [kernel-hardening] " Kees Cook ` (2 more replies) 2016-10-21 7:47 ` [kernel-hardening] Re: [RFC 0/2] arm: implementation of HARDENED_ATOMIC AKASHI Takahiro 2016-10-27 10:32 ` [kernel-hardening] " Mark Rutland 3 siblings, 3 replies; 30+ messages in thread From: Colin Vidal @ 2016-10-18 14:59 UTC (permalink / raw) To: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand Cc: Colin Vidal 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@cvidal.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) + #define ATOMIC_FETCH_OP(op, c_op, asm_op) \ static inline int atomic_fetch_##op##_relaxed(int i, atomic_t *v) \ { \ @@ -108,26 +155,34 @@ static inline int atomic_fetch_##op##_relaxed(int i, atomic_t *v) \ #define atomic_fetch_or_relaxed atomic_fetch_or_relaxed #define atomic_fetch_xor_relaxed atomic_fetch_xor_relaxed -static inline int atomic_cmpxchg_relaxed(atomic_t *ptr, int old, int new) -{ - int oldval; - unsigned long res; - - prefetchw(&ptr->counter); - - do { - __asm__ __volatile__("@ atomic_cmpxchg\n" - "ldrex %1, [%3]\n" - "mov %0, #0\n" - "teq %1, %4\n" - "strexeq %0, %5, [%3]\n" - : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) - : "r" (&ptr->counter), "Ir" (old), "r" (new) - : "cc"); - } while (res); - - return oldval; +#define __ATOMIC_CMPXCHG_RELAXED(suffix) \ +static inline int atomic_cmpxchg##suffix##_relaxed(atomic##suffix##_t *ptr, \ + int old, int new) \ +{ \ + int oldval; \ + unsigned long res; \ + \ + prefetchw(&ptr->counter); \ + \ + do { \ + __asm__ __volatile__("@ atomic_cmpxchg" #suffix "\n" \ + "ldrex %1, [%3]\n" \ + "mov %0, #0\n" \ + "teq %1, %4\n" \ + "strexeq %0, %5, [%3]\n" \ + : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) \ + : "r" (&ptr->counter), "Ir" (old), "r" (new) \ + : "cc"); \ + } while (res); \ + \ + return oldval; \ } + +__ATOMIC_CMPXCHG_RELAXED() +__ATOMIC_CMPXCHG_RELAXED(_wrap) + +#undef __ATOMIC_CMPXCHG_RELAXED + #define atomic_cmpxchg_relaxed atomic_cmpxchg_relaxed static inline int __atomic_add_unless(atomic_t *v, int a, int u) @@ -141,12 +196,21 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) __asm__ __volatile__ ("@ atomic_add_unless\n" "1: ldrex %0, [%4]\n" " teq %0, %5\n" -" beq 2f\n" -" add %1, %0, %6\n" +" beq 4f\n" +" adds %1, %0, %6\n" + +#ifdef CONFIG_HARDENED_ATOMIC +" bvc 3f\n" +"2: "HARDENED_ATOMIC_INSN"\n" +"3:\n" +#endif " strex %2, %1, [%4]\n" " teq %2, #0\n" " bne 1b\n" -"2:" +"4:" +#ifdef CONFIG_HARDENED_ATOMIC + _ASM_EXTABLE(2b, 4b) +#endif : "=&r" (oldval), "=&r" (newval), "=&r" (tmp), "+Qo" (v->counter) : "r" (&v->counter), "r" (u), "r" (a) : "cc"); @@ -163,8 +227,8 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) #error SMP not supported on pre-ARMv6 CPUs #endif -#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) \ +static inline void atomic_##op##suffix(int i, atomic##suffix##_t *v) \ { \ unsigned long flags; \ \ @@ -173,8 +237,12 @@ static inline void atomic_##op(int i, atomic_t *v) \ raw_local_irq_restore(flags); \ } \ -#define ATOMIC_OP_RETURN(op, c_op, asm_op) \ -static inline int atomic_##op##_return(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) + +#define __ATOMIC_OP_RETURN(op, suffix, c_op, asm_op) \ +static inline int atomic_##op##_return##suffix(int i, atomic##suffix##_t *v) \ { \ unsigned long flags; \ int val; \ @@ -187,6 +255,10 @@ static inline int atomic_##op##_return(int i, atomic_t *v) \ return val; \ } +#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) + #define ATOMIC_FETCH_OP(op, c_op, asm_op) \ static inline int atomic_fetch_##op(int i, atomic_t *v) \ { \ @@ -215,6 +287,11 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int new) return ret; } +static inline int atomic_cmpxchg_wrap(atomic_wrap_t *v, int old, int new) +{ + return atomic_cmpxchg((atomic_t *)v, old, new); +} + static inline int __atomic_add_unless(atomic_t *v, int a, int u) { int c, old; @@ -227,6 +304,11 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) #endif /* __LINUX_ARM_ARCH__ */ +static inline int __atomic_add_unless_wrap(atomic_wrap_t *v, int a, int u) +{ + return __atomic_add_unless((atomic_t *)v, a, u); +} + #define ATOMIC_OPS(op, c_op, asm_op) \ ATOMIC_OP(op, c_op, asm_op) \ ATOMIC_OP_RETURN(op, c_op, asm_op) \ @@ -250,18 +332,30 @@ ATOMIC_OPS(xor, ^=, eor) #undef ATOMIC_OPS #undef ATOMIC_FETCH_OP #undef ATOMIC_OP_RETURN +#undef __ATOMIC_OP_RETURN #undef ATOMIC_OP +#undef __ATOMIC_OP #define atomic_xchg(v, new) (xchg(&((v)->counter), new)) - +#define atomic_xchg_wrap(v, new) atomic_xchg(v, new) #define atomic_inc(v) atomic_add(1, v) +static inline void atomic_inc_wrap(atomic_wrap_t *v) +{ + atomic_add_wrap(1, v); +} #define atomic_dec(v) atomic_sub(1, v) +static inline void atomic_dec_wrap(atomic_wrap_t *v) +{ + atomic_sub_wrap(1, v); +} #define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0) #define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0) #define atomic_inc_return_relaxed(v) (atomic_add_return_relaxed(1, v)) +#define atomic_inc_return_wrap_relaxed(v) (atomic_add_return_wrap_relaxed(1, v)) #define atomic_dec_return_relaxed(v) (atomic_sub_return_relaxed(1, v)) #define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0) +#define atomic_sub_and_test_wrap(i, v) (atomic_sub_return_wrap(i, v) == 0) #define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0) @@ -270,62 +364,81 @@ typedef struct { long long counter; } atomic64_t; +#ifdef CONFIG_HARDENED_ATOMIC +typedef struct { + long long counter; +} atomic64_wrap_t; +#else +typedef atomic64_t atomic64_wrap_t; +#endif + #define ATOMIC64_INIT(i) { (i) } -#ifdef CONFIG_ARM_LPAE -static inline long long atomic64_read(const atomic64_t *v) -{ - long long result; +#define __ATOMIC64_READ(suffix, asm_op) \ +static inline long long \ +atomic64_read##suffix(const atomic64##suffix##_t *v) \ +{ \ + long long result; \ + \ + __asm__ __volatile__("@ atomic64_read" #suffix "\n" \ +" " #asm_op " %0, %H0, [%1]" \ + : "=&r" (result) \ + : "r" (&v->counter), "Qo" (v->counter) \ + ); \ + \ + return result; \ +} - __asm__ __volatile__("@ atomic64_read\n" -" ldrd %0, %H0, [%1]" - : "=&r" (result) - : "r" (&v->counter), "Qo" (v->counter) - ); +#ifdef CONFIG_ARM_LPAE +__ATOMIC64_READ(, ldrd) +__ATOMIC64_READ(wrap, ldrd) - return result; +#define __ATOMIC64_SET(suffix) \ +static inline void atomic64_set##suffix(atomic64##suffix##_t *v, long long i) \ +{ \ + __asm__ __volatile__("@ atomic64_set" #suffix "\n" \ +" strd %2, %H2, [%1]" \ + : "=Qo" (v->counter) \ + : "r" (&v->counter), "r" (i) \ + ); \ } -static inline void atomic64_set(atomic64_t *v, long long i) -{ - __asm__ __volatile__("@ atomic64_set\n" -" strd %2, %H2, [%1]" - : "=Qo" (v->counter) - : "r" (&v->counter), "r" (i) - ); -} -#else -static inline long long atomic64_read(const atomic64_t *v) -{ - long long result; +__ATOMIC64_SET() +__ATOMIC64_SET(_wrap) - __asm__ __volatile__("@ atomic64_read\n" -" ldrexd %0, %H0, [%1]" - : "=&r" (result) - : "r" (&v->counter), "Qo" (v->counter) - ); +#undef __ATOMIC64 - return result; +#else +__ATOMIC64_READ(, ldrexd) +__ATOMIC64_READ(_wrap, ldrexd) + +#define __ATOMIC64_SET(suffix) \ +static inline void atomic64_set##suffix(atomic64##suffix##_t *v, long long i) \ +{ \ + long long tmp; \ + \ + prefetchw(&v->counter); \ + __asm__ __volatile__("@ atomic64_set" #suffix"\n" \ +"1: ldrexd %0, %H0, [%2]\n" \ +" strexd %0, %3, %H3, [%2]\n" \ +" teq %0, #0\n" \ +" bne 1b" \ + : "=&r" (tmp), "=Qo" (v->counter) \ + : "r" (&v->counter), "r" (i) \ + : "cc"); \ } -static inline void atomic64_set(atomic64_t *v, long long i) -{ - long long tmp; +__ATOMIC64_SET() +__ATOMIC64_SET(_wrap) + +#undef __ATOMIC64_SET - prefetchw(&v->counter); - __asm__ __volatile__("@ atomic64_set\n" -"1: ldrexd %0, %H0, [%2]\n" -" strexd %0, %3, %H3, [%2]\n" -" teq %0, #0\n" -" bne 1b" - : "=&r" (tmp), "=Qo" (v->counter) - : "r" (&v->counter), "r" (i) - : "cc"); -} #endif -#define ATOMIC64_OP(op, op1, op2) \ -static inline void atomic64_##op(long long i, atomic64_t *v) \ +#undef __ATOMIC64_READ + +#define __ATOMIC64_OP(op, suffix, op1, op2, post_op, extable) \ +static inline void atomic64_##op##suffix(long long i, atomic64##suffix##_t *v) \ { \ long long result; \ unsigned long tmp; \ @@ -335,17 +448,31 @@ static inline void atomic64_##op(long long i, atomic64_t *v) \ "1: ldrexd %0, %H0, [%3]\n" \ " " #op1 " %Q0, %Q0, %Q4\n" \ " " #op2 " %R0, %R0, %R4\n" \ + post_op \ " strexd %1, %0, %H0, [%3]\n" \ " teq %1, #0\n" \ -" bne 1b" \ +" bne 1b\n" \ + extable \ : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) \ : "r" (&v->counter), "r" (i) \ : "cc"); \ -} \ +} -#define ATOMIC64_OP_RETURN(op, op1, op2) \ +#define ATOMIC64_OP(op, op1, op2) \ + __ATOMIC64_OP(op, _wrap, op1, op2, , ) \ + __ATOMIC64_OP(op, , op1, op2##s, __OVERFLOW_POST, __OVERFLOW_EXTABLE) + +#undef __OVERFLOW_POST_RETURN +#define __OVERFLOW_POST_RETURN \ + "bvc 3f\n" \ + "mov %0, %1\n" \ + "mov %H0, %H1\n" \ + "2: "HARDENED_ATOMIC_INSN"\n" \ + "3:\n" + +#define __ATOMIC64_OP_RETURN(op, suffix, op1, op2, post_op, extable) \ static inline long long \ -atomic64_##op##_return_relaxed(long long i, atomic64_t *v) \ +atomic64_##op##_return##suffix##_relaxed(long long i, atomic64##suffix##_t *v) \ { \ long long result; \ unsigned long tmp; \ @@ -356,9 +483,11 @@ atomic64_##op##_return_relaxed(long long i, atomic64_t *v) \ "1: ldrexd %0, %H0, [%3]\n" \ " " #op1 " %Q0, %Q0, %Q4\n" \ " " #op2 " %R0, %R0, %R4\n" \ + post_op \ " strexd %1, %0, %H0, [%3]\n" \ " teq %1, #0\n" \ -" bne 1b" \ +" bne 1b\n" \ + extable \ : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) \ : "r" (&v->counter), "r" (i) \ : "cc"); \ @@ -366,6 +495,11 @@ atomic64_##op##_return_relaxed(long long i, atomic64_t *v) \ return result; \ } +#define ATOMIC64_OP_RETURN(op, op1, op2) \ + __ATOMIC64_OP_RETURN(op, _wrap, op1, op2, , ) \ + __ATOMIC64_OP_RETURN(op, , op1, op2##s, __OVERFLOW_POST_RETURN, \ + __OVERFLOW_EXTABLE) + #define ATOMIC64_FETCH_OP(op, op1, op2) \ static inline long long \ atomic64_fetch_##op##_relaxed(long long i, atomic64_t *v) \ @@ -422,70 +556,98 @@ ATOMIC64_OPS(xor, eor, eor) #undef ATOMIC64_OPS #undef ATOMIC64_FETCH_OP #undef ATOMIC64_OP_RETURN +#undef __ATOMIC64_OP_RETURN #undef ATOMIC64_OP - -static inline long long -atomic64_cmpxchg_relaxed(atomic64_t *ptr, long long old, long long new) -{ - long long oldval; - unsigned long res; - - prefetchw(&ptr->counter); - - do { - __asm__ __volatile__("@ atomic64_cmpxchg\n" - "ldrexd %1, %H1, [%3]\n" - "mov %0, #0\n" - "teq %1, %4\n" - "teqeq %H1, %H4\n" - "strexdeq %0, %5, %H5, [%3]" - : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) - : "r" (&ptr->counter), "r" (old), "r" (new) - : "cc"); - } while (res); - - return oldval; +#undef __ATOMIC64_OP +#undef __OVERFLOW_EXTABLE +#undef __OVERFLOW_POST_RETURN +#undef __OVERFLOW_RETURN + +#define __ATOMIC64_CMPXCHG_RELAXED(suffix) \ +static inline long long atomic64_cmpxchg##suffix##_relaxed( \ + atomic64##suffix##_t *ptr, long long old, long long new) \ +{ \ + long long oldval; \ + unsigned long res; \ + \ + prefetchw(&ptr->counter); \ + \ + do { \ + __asm__ __volatile__("@ atomic64_cmpxchg" #suffix "\n" \ + "ldrexd %1, %H1, [%3]\n" \ + "mov %0, #0\n" \ + "teq %1, %4\n" \ + "teqeq %H1, %H4\n" \ + "strexdeq %0, %5, %H5, [%3]" \ + : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) \ + : "r" (&ptr->counter), "r" (old), "r" (new) \ + : "cc"); \ + } while (res); \ + \ + return oldval; \ } -#define atomic64_cmpxchg_relaxed atomic64_cmpxchg_relaxed -static inline long long atomic64_xchg_relaxed(atomic64_t *ptr, long long new) -{ - long long result; - unsigned long tmp; - - prefetchw(&ptr->counter); +__ATOMIC64_CMPXCHG_RELAXED() +__ATOMIC64_CMPXCHG_RELAXED(_wrap) +#define atomic64_cmpxchg_relaxed atomic64_cmpxchg_relaxed - __asm__ __volatile__("@ atomic64_xchg\n" -"1: ldrexd %0, %H0, [%3]\n" -" strexd %1, %4, %H4, [%3]\n" -" teq %1, #0\n" -" bne 1b" - : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter) - : "r" (&ptr->counter), "r" (new) - : "cc"); +#undef __ATOMIC64_CMPXCHG_RELAXED - return result; +#define __ATOMIC64_XCHG_RELAXED(suffix) \ +static inline long long atomic64_xchg##suffix##_relaxed( \ + atomic64##suffix##_t *ptr, long long new) \ +{ \ + long long result; \ + unsigned long tmp; \ + \ + prefetchw(&ptr->counter); \ + \ + __asm__ __volatile__("@ atomic64_xchg" #suffix "\n" \ +"1: ldrexd %0, %H0, [%3]\n" \ +" strexd %1, %4, %H4, [%3]\n" \ +" teq %1, #0\n" \ +" bne 1b" \ + : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter) \ + : "r" (&ptr->counter), "r" (new) \ + : "cc"); \ + \ + return result; \ } + +__ATOMIC64_XCHG_RELAXED() +__ATOMIC64_XCHG_RELAXED(_wrap) #define atomic64_xchg_relaxed atomic64_xchg_relaxed +#undef __ATOMIC64_XCHG_RELAXED + static inline long long atomic64_dec_if_positive(atomic64_t *v) { long long result; - unsigned long tmp; + u64 tmp; smp_mb(); prefetchw(&v->counter); __asm__ __volatile__("@ atomic64_dec_if_positive\n" -"1: ldrexd %0, %H0, [%3]\n" -" subs %Q0, %Q0, #1\n" -" sbc %R0, %R0, #0\n" +"1: ldrexd %1, %H1, [%3]\n" +" subs %Q0, %Q1, #1\n" +" sbcs %R0, %R1, #0\n" +#ifdef CONFIG_HARDENED_ATOMIC +" bvc 3f\n" +" mov %Q0, %Q1\n" +" mov %R0, %R1\n" +"2: "HARDENED_ATOMIC_INSN"\n" +"3:\n" +#endif " teq %R0, #0\n" -" bmi 2f\n" +" bmi 4f\n" " strexd %1, %0, %H0, [%3]\n" " teq %1, #0\n" " bne 1b\n" -"2:" +"4:\n" +#ifdef CONFIG_HARDENED_ATOMIC + _ASM_EXTABLE(2b, 4b) +#endif : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) : "r" (&v->counter) : "cc"); @@ -509,13 +671,21 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u) " teq %0, %5\n" " teqeq %H0, %H5\n" " moveq %1, #0\n" -" beq 2f\n" +" beq 4f\n" " adds %Q0, %Q0, %Q6\n" -" adc %R0, %R0, %R6\n" +" adcs %R0, %R0, %R6\n" +#ifdef CONFIG_HARDENED_ATOMIC +" bvc 3f\n" +"2: "HARDENED_ATOMIC_INSN"\n" +"3:\n" +#endif " strexd %2, %0, %H0, [%4]\n" " teq %2, #0\n" " bne 1b\n" -"2:" +"4:\n" +#ifdef CONFIG_HARDENED_ATOMIC + _ASM_EXTABLE(2b, 4b) +#endif : "=&r" (val), "+r" (ret), "=&r" (tmp), "+Qo" (v->counter) : "r" (&v->counter), "r" (u), "r" (a) : "cc"); @@ -529,6 +699,7 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u) #define atomic64_add_negative(a, v) (atomic64_add_return((a), (v)) < 0) #define atomic64_inc(v) atomic64_add(1LL, (v)) #define atomic64_inc_return_relaxed(v) atomic64_add_return_relaxed(1LL, (v)) +#define atomic64_inc_return_wrap_relaxed(v) atomic64_add_return_wrap_relaxed(1LL, v) #define atomic64_inc_and_test(v) (atomic64_inc_return(v) == 0) #define atomic64_sub_and_test(a, v) (atomic64_sub_return((a), (v)) == 0) #define atomic64_dec(v) atomic64_sub(1LL, (v)) @@ -536,6 +707,9 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u) #define atomic64_dec_and_test(v) (atomic64_dec_return((v)) == 0) #define atomic64_inc_not_zero(v) atomic64_add_unless((v), 1LL, 0LL) +#define atomic64_inc_wrap(v) atomic64_add_wrap(1LL, v) +#define atomic64_dec_wrap(v) atomic64_sub_wrap(1LL, v) + #endif /* !CONFIG_GENERIC_ATOMIC64 */ #endif #endif diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index 3a2e678..ce8ee00 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); struct siginfo info; +#ifdef CONFIG_HARDENED_ATOMIC + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { + unsigned long pc = instruction_pointer(regs); + unsigned int bkpt; + + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && + cpu_to_le32(bkpt) == 0xe12f1073) { + current->thread.error_code = ifsr; + current->thread.trap_no = 0; + hardened_atomic_overflow(regs); + fixup_exception(regs); + return; + } + } +#endif if (!inf->fn(addr, ifsr | FSR_LNX_PF, regs)) return; -- 2.7.4 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-18 14:59 ` [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC Colin Vidal @ 2016-10-18 21:29 ` Kees Cook 2016-10-19 8:45 ` Colin Vidal 2016-10-25 9:18 ` AKASHI Takahiro 2016-10-27 13:24 ` [kernel-hardening] " Mark Rutland 2 siblings, 1 reply; 30+ messages in thread From: Kees Cook @ 2016-10-18 21:29 UTC (permalink / raw) To: Colin Vidal Cc: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Hans Liljestrand On Tue, Oct 18, 2016 at 7:59 AM, Colin Vidal <colin@cvidal.org> 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. Can you include some notes that this was originally in PaX/grsecurity, and detail what is different from their implemention? > > Signed-off-by: Colin Vidal <colin@cvidal.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" In PaX, I see a check for THUMB2 config: #ifdef CONFIG_THUMB2_KERNEL #define REFCOUNT_TRAP_INSN "bkpt 0xf1" #else #define REFCOUNT_TRAP_INSN "bkpt 0xf103" #endif That should probably stay unless I'm misunderstanding something. Also, for your new ISNS define name, I'd leave "TRAP" in the name, since that describes more clearly what it does. Beyond these things, it looks great to me -- though I'm hardly an ARM expert. :) Were you able to test on ARM with this for overflow with the lkdtm tests? -Kees -- Kees Cook Nexus Security ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-18 21:29 ` [kernel-hardening] " Kees Cook @ 2016-10-19 8:45 ` Colin Vidal 2016-10-19 20:11 ` Kees Cook 0 siblings, 1 reply; 30+ messages in thread From: Colin Vidal @ 2016-10-19 8:45 UTC (permalink / raw) To: Kees Cook Cc: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Hans Liljestrand Hi Kees, > > 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. > > Can you include some notes that this was originally in PaX/grsecurity, > and detail what is different from their implemention? Of course. I add it in the next version. > > Signed-off-by: Colin Vidal <colin@cvidal.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" > > In PaX, I see a check for THUMB2 config: > > #ifdef CONFIG_THUMB2_KERNEL > #define REFCOUNT_TRAP_INSN "bkpt 0xf1" > #else > #define REFCOUNT_TRAP_INSN "bkpt 0xf103" > #endif > > That should probably stay unless I'm misunderstanding something. Also, > for your new ISNS define name, I'd leave "TRAP" in the name, since > that describes more clearly what it does. Oh yeah. I will add it. Actually I does not add it at first since I does not really understand why there is a special case for Thumbs2 (as far I understand, instructions size can also be 4 bytes). If ARM experts are around, I would appreciate pointers about it :-) > Beyond these things, it looks great to me -- though I'm hardly an ARM expert. :) > > Were you able to test on ARM with this for overflow with the lkdtm tests? Yep. I have to make more thorough tests, but things like echo HARDENED_ATOMIC_OVERFLOW > <debugfsmountpoint>/provoke-crash/DIRECT raise the exception as well (with hardened message and stack trace). I tested it with armv7/qemu. Thanks! Colin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-19 8:45 ` Colin Vidal @ 2016-10-19 20:11 ` Kees Cook 2016-10-20 5:58 ` AKASHI Takahiro 0 siblings, 1 reply; 30+ messages in thread From: Kees Cook @ 2016-10-19 20:11 UTC (permalink / raw) To: Colin Vidal Cc: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Hans Liljestrand, Mark Rutland On Wed, Oct 19, 2016 at 1:45 AM, Colin Vidal <colin@cvidal.org> wrote: > Hi Kees, > >> > 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. >> >> Can you include some notes that this was originally in PaX/grsecurity, >> and detail what is different from their implemention? > > Of course. I add it in the next version. > >> > Signed-off-by: Colin Vidal <colin@cvidal.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" >> >> In PaX, I see a check for THUMB2 config: >> >> #ifdef CONFIG_THUMB2_KERNEL >> #define REFCOUNT_TRAP_INSN "bkpt 0xf1" >> #else >> #define REFCOUNT_TRAP_INSN "bkpt 0xf103" >> #endif >> >> That should probably stay unless I'm misunderstanding something. Also, >> for your new ISNS define name, I'd leave "TRAP" in the name, since >> that describes more clearly what it does. > > Oh yeah. I will add it. Actually I does not add it at first since I > does not really understand why there is a special case for Thumbs2 (as > far I understand, instructions size can also be 4 bytes). If ARM > experts are around, I would appreciate pointers about it :-) Cool. Perhaps Takahiro or Mark (now on CC) can comment on it. >> Beyond these things, it looks great to me -- though I'm hardly an ARM expert. :) >> >> Were you able to test on ARM with this for overflow with the lkdtm tests? > > Yep. I have to make more thorough tests, but things like > > echo HARDENED_ATOMIC_OVERFLOW > <debugfsmountpoint>/provoke-crash/DIRECT > > raise the exception as well (with hardened message and stack trace). I > tested it with armv7/qemu. Great! -Kees -- Kees Cook Nexus Security ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-19 20:11 ` Kees Cook @ 2016-10-20 5:58 ` AKASHI Takahiro 2016-10-20 8:30 ` Colin Vidal 0 siblings, 1 reply; 30+ messages in thread From: AKASHI Takahiro @ 2016-10-20 5:58 UTC (permalink / raw) To: Kees Cook Cc: Colin Vidal, kernel-hardening, Reshetova, Elena, David Windsor, Hans Liljestrand, Mark Rutland On Wed, Oct 19, 2016 at 01:11:46PM -0700, Kees Cook wrote: > On Wed, Oct 19, 2016 at 1:45 AM, Colin Vidal <colin@cvidal.org> wrote: > > Hi Kees, > > > >> > 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. > >> > >> Can you include some notes that this was originally in PaX/grsecurity, > >> and detail what is different from their implemention? > > > > Of course. I add it in the next version. > > > >> > Signed-off-by: Colin Vidal <colin@cvidal.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" > >> > >> In PaX, I see a check for THUMB2 config: > >> > >> #ifdef CONFIG_THUMB2_KERNEL > >> #define REFCOUNT_TRAP_INSN "bkpt 0xf1" > >> #else > >> #define REFCOUNT_TRAP_INSN "bkpt 0xf103" > >> #endif > >> > >> That should probably stay unless I'm misunderstanding something. Also, > >> for your new ISNS define name, I'd leave "TRAP" in the name, since > >> that describes more clearly what it does. > > > > Oh yeah. I will add it. Actually I does not add it at first since I > > does not really understand why there is a special case for Thumbs2 (as > > far I understand, instructions size can also be 4 bytes). If ARM > > experts are around, I would appreciate pointers about it :-) > > Cool. Perhaps Takahiro or Mark (now on CC) can comment on it. "bkpt" is a 16-bit instruction with an 8-bit immediate value in thumb2. -Takahiro AKASHI > >> Beyond these things, it looks great to me -- though I'm hardly an ARM expert. :) > >> > >> Were you able to test on ARM with this for overflow with the lkdtm tests? > > > > Yep. I have to make more thorough tests, but things like > > > > echo HARDENED_ATOMIC_OVERFLOW > <debugfsmountpoint>/provoke-crash/DIRECT > > > > raise the exception as well (with hardened message and stack trace). I > > tested it with armv7/qemu. > > Great! > > -Kees > > -- > Kees Cook > Nexus Security ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-20 5:58 ` AKASHI Takahiro @ 2016-10-20 8:30 ` Colin Vidal 0 siblings, 0 replies; 30+ messages in thread From: Colin Vidal @ 2016-10-20 8:30 UTC (permalink / raw) To: kernel-hardening, Kees Cook Cc: Reshetova, Elena, David Windsor, Hans Liljestrand, Mark Rutland On Thu, 2016-10-20 at 14:58 +0900, AKASHI Takahiro wrote: > On Wed, Oct 19, 2016 at 01:11:46PM -0700, Kees Cook wrote: > > > > On Wed, Oct 19, 2016 at 1:45 AM, Colin Vidal <colin@cvidal.org> wrote: > > > > > > Hi Kees, > > > > > > > > > > > > > > > > > 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. > > > > > > > > Can you include some notes that this was originally in PaX/grsecurity, > > > > and detail what is different from their implemention? > > > > > > Of course. I add it in the next version. > > > > > > > > > > > > > > > > > Signed-off-by: Colin Vidal <colin@cvidal.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" > > > > > > > > In PaX, I see a check for THUMB2 config: > > > > > > > > #ifdef CONFIG_THUMB2_KERNEL > > > > #define REFCOUNT_TRAP_INSN "bkpt 0xf1" > > > > #else > > > > #define REFCOUNT_TRAP_INSN "bkpt 0xf103" > > > > #endif > > > > > > > > That should probably stay unless I'm misunderstanding something. Also, > > > > for your new ISNS define name, I'd leave "TRAP" in the name, since > > > > that describes more clearly what it does. > > > > > > Oh yeah. I will add it. Actually I does not add it at first since I > > > does not really understand why there is a special case for Thumbs2 (as > > > far I understand, instructions size can also be 4 bytes). If ARM > > > experts are around, I would appreciate pointers about it :-) > > > > Cool. Perhaps Takahiro or Mark (now on CC) can comment on it. > > "bkpt" is a 16-bit instruction with an 8-bit immediate value in thumb2. Ok, I better understand why PaX uses this guard now. Thanks! Colin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-18 14:59 ` [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC Colin Vidal 2016-10-18 21:29 ` [kernel-hardening] " Kees Cook @ 2016-10-25 9:18 ` AKASHI Takahiro 2016-10-25 15:02 ` Colin Vidal 2016-10-27 13:24 ` [kernel-hardening] " Mark Rutland 2 siblings, 1 reply; 30+ messages in thread From: AKASHI Takahiro @ 2016-10-25 9:18 UTC (permalink / raw) To: Colin Vidal Cc: kernel-hardening, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand 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@cvidal.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 > #define ATOMIC_FETCH_OP(op, c_op, asm_op) \ > static inline int atomic_fetch_##op##_relaxed(int i, atomic_t *v) \ > { \ > @@ -108,26 +155,34 @@ static inline int atomic_fetch_##op##_relaxed(int i, atomic_t *v) \ > #define atomic_fetch_or_relaxed atomic_fetch_or_relaxed > #define atomic_fetch_xor_relaxed atomic_fetch_xor_relaxed > > -static inline int atomic_cmpxchg_relaxed(atomic_t *ptr, int old, int new) > -{ > - int oldval; > - unsigned long res; > - > - prefetchw(&ptr->counter); > - > - do { > - __asm__ __volatile__("@ atomic_cmpxchg\n" > - "ldrex %1, [%3]\n" > - "mov %0, #0\n" > - "teq %1, %4\n" > - "strexeq %0, %5, [%3]\n" > - : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) > - : "r" (&ptr->counter), "Ir" (old), "r" (new) > - : "cc"); > - } while (res); > - > - return oldval; > +#define __ATOMIC_CMPXCHG_RELAXED(suffix) \ > +static inline int atomic_cmpxchg##suffix##_relaxed(atomic##suffix##_t *ptr, \ > + int old, int new) \ > +{ \ > + int oldval; \ > + unsigned long res; \ > + \ > + prefetchw(&ptr->counter); \ > + \ > + do { \ > + __asm__ __volatile__("@ atomic_cmpxchg" #suffix "\n" \ > + "ldrex %1, [%3]\n" \ > + "mov %0, #0\n" \ > + "teq %1, %4\n" \ > + "strexeq %0, %5, [%3]\n" \ > + : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) \ > + : "r" (&ptr->counter), "Ir" (old), "r" (new) \ > + : "cc"); \ > + } while (res); \ > + \ > + return oldval; \ > } > + > +__ATOMIC_CMPXCHG_RELAXED() > +__ATOMIC_CMPXCHG_RELAXED(_wrap) > + > +#undef __ATOMIC_CMPXCHG_RELAXED > + > #define atomic_cmpxchg_relaxed atomic_cmpxchg_relaxed > > static inline int __atomic_add_unless(atomic_t *v, int a, int u) > @@ -141,12 +196,21 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) > __asm__ __volatile__ ("@ atomic_add_unless\n" > "1: ldrex %0, [%4]\n" > " teq %0, %5\n" > -" beq 2f\n" > -" add %1, %0, %6\n" > +" beq 4f\n" > +" adds %1, %0, %6\n" > + > +#ifdef CONFIG_HARDENED_ATOMIC > +" bvc 3f\n" > +"2: "HARDENED_ATOMIC_INSN"\n" > +"3:\n" > +#endif > " strex %2, %1, [%4]\n" > " teq %2, #0\n" > " bne 1b\n" > -"2:" > +"4:" > +#ifdef CONFIG_HARDENED_ATOMIC > + _ASM_EXTABLE(2b, 4b) > +#endif > : "=&r" (oldval), "=&r" (newval), "=&r" (tmp), "+Qo" (v->counter) > : "r" (&v->counter), "r" (u), "r" (a) > : "cc"); > @@ -163,8 +227,8 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) > #error SMP not supported on pre-ARMv6 CPUs > #endif > > -#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) \ > +static inline void atomic_##op##suffix(int i, atomic##suffix##_t *v) \ > { \ > unsigned long flags; \ > \ > @@ -173,8 +237,12 @@ static inline void atomic_##op(int i, atomic_t *v) \ > raw_local_irq_restore(flags); \ > } \ > > -#define ATOMIC_OP_RETURN(op, c_op, asm_op) \ > -static inline int atomic_##op##_return(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) > + > +#define __ATOMIC_OP_RETURN(op, suffix, c_op, asm_op) \ > +static inline int atomic_##op##_return##suffix(int i, atomic##suffix##_t *v) \ > { \ > unsigned long flags; \ > int val; \ > @@ -187,6 +255,10 @@ static inline int atomic_##op##_return(int i, atomic_t *v) \ > return val; \ > } > > +#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) > + > #define ATOMIC_FETCH_OP(op, c_op, asm_op) \ > static inline int atomic_fetch_##op(int i, atomic_t *v) \ > { \ > @@ -215,6 +287,11 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int new) > return ret; > } > > +static inline int atomic_cmpxchg_wrap(atomic_wrap_t *v, int old, int new) > +{ > + return atomic_cmpxchg((atomic_t *)v, old, new); > +} > + > static inline int __atomic_add_unless(atomic_t *v, int a, int u) > { > int c, old; > @@ -227,6 +304,11 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) > > #endif /* __LINUX_ARM_ARCH__ */ > > +static inline int __atomic_add_unless_wrap(atomic_wrap_t *v, int a, int u) > +{ > + return __atomic_add_unless((atomic_t *)v, a, u); > +} > + > #define ATOMIC_OPS(op, c_op, asm_op) \ > ATOMIC_OP(op, c_op, asm_op) \ > ATOMIC_OP_RETURN(op, c_op, asm_op) \ > @@ -250,18 +332,30 @@ ATOMIC_OPS(xor, ^=, eor) > #undef ATOMIC_OPS > #undef ATOMIC_FETCH_OP > #undef ATOMIC_OP_RETURN > +#undef __ATOMIC_OP_RETURN > #undef ATOMIC_OP > +#undef __ATOMIC_OP > > #define atomic_xchg(v, new) (xchg(&((v)->counter), new)) > - > +#define atomic_xchg_wrap(v, new) atomic_xchg(v, new) > #define atomic_inc(v) atomic_add(1, v) > +static inline void atomic_inc_wrap(atomic_wrap_t *v) > +{ > + atomic_add_wrap(1, v); > +} > #define atomic_dec(v) atomic_sub(1, v) > +static inline void atomic_dec_wrap(atomic_wrap_t *v) > +{ > + atomic_sub_wrap(1, v); > +} > > #define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0) > #define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0) > #define atomic_inc_return_relaxed(v) (atomic_add_return_relaxed(1, v)) > +#define atomic_inc_return_wrap_relaxed(v) (atomic_add_return_wrap_relaxed(1, v)) > #define atomic_dec_return_relaxed(v) (atomic_sub_return_relaxed(1, v)) > #define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0) > +#define atomic_sub_and_test_wrap(i, v) (atomic_sub_return_wrap(i, v) == 0) > > #define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0) > > @@ -270,62 +364,81 @@ typedef struct { > long long counter; > } atomic64_t; > > +#ifdef CONFIG_HARDENED_ATOMIC > +typedef struct { > + long long counter; > +} atomic64_wrap_t; > +#else > +typedef atomic64_t atomic64_wrap_t; > +#endif > + > #define ATOMIC64_INIT(i) { (i) } > > -#ifdef CONFIG_ARM_LPAE > -static inline long long atomic64_read(const atomic64_t *v) > -{ > - long long result; > +#define __ATOMIC64_READ(suffix, asm_op) \ > +static inline long long \ > +atomic64_read##suffix(const atomic64##suffix##_t *v) \ > +{ \ > + long long result; \ > + \ > + __asm__ __volatile__("@ atomic64_read" #suffix "\n" \ > +" " #asm_op " %0, %H0, [%1]" \ > + : "=&r" (result) \ > + : "r" (&v->counter), "Qo" (v->counter) \ > + ); \ > + \ > + return result; \ > +} > > - __asm__ __volatile__("@ atomic64_read\n" > -" ldrd %0, %H0, [%1]" > - : "=&r" (result) > - : "r" (&v->counter), "Qo" (v->counter) > - ); > +#ifdef CONFIG_ARM_LPAE > +__ATOMIC64_READ(, ldrd) > +__ATOMIC64_READ(wrap, ldrd) > > - return result; > +#define __ATOMIC64_SET(suffix) \ > +static inline void atomic64_set##suffix(atomic64##suffix##_t *v, long long i) \ > +{ \ > + __asm__ __volatile__("@ atomic64_set" #suffix "\n" \ > +" strd %2, %H2, [%1]" \ > + : "=Qo" (v->counter) \ > + : "r" (&v->counter), "r" (i) \ > + ); \ > } > > -static inline void atomic64_set(atomic64_t *v, long long i) > -{ > - __asm__ __volatile__("@ atomic64_set\n" > -" strd %2, %H2, [%1]" > - : "=Qo" (v->counter) > - : "r" (&v->counter), "r" (i) > - ); > -} > -#else > -static inline long long atomic64_read(const atomic64_t *v) > -{ > - long long result; > +__ATOMIC64_SET() > +__ATOMIC64_SET(_wrap) > > - __asm__ __volatile__("@ atomic64_read\n" > -" ldrexd %0, %H0, [%1]" > - : "=&r" (result) > - : "r" (&v->counter), "Qo" (v->counter) > - ); > +#undef __ATOMIC64 > > - return result; > +#else > +__ATOMIC64_READ(, ldrexd) > +__ATOMIC64_READ(_wrap, ldrexd) > + > +#define __ATOMIC64_SET(suffix) \ > +static inline void atomic64_set##suffix(atomic64##suffix##_t *v, long long i) \ > +{ \ > + long long tmp; \ > + \ > + prefetchw(&v->counter); \ > + __asm__ __volatile__("@ atomic64_set" #suffix"\n" \ > +"1: ldrexd %0, %H0, [%2]\n" \ > +" strexd %0, %3, %H3, [%2]\n" \ > +" teq %0, #0\n" \ > +" bne 1b" \ > + : "=&r" (tmp), "=Qo" (v->counter) \ > + : "r" (&v->counter), "r" (i) \ > + : "cc"); \ > } > > -static inline void atomic64_set(atomic64_t *v, long long i) > -{ > - long long tmp; > +__ATOMIC64_SET() > +__ATOMIC64_SET(_wrap) > + > +#undef __ATOMIC64_SET > > - prefetchw(&v->counter); > - __asm__ __volatile__("@ atomic64_set\n" > -"1: ldrexd %0, %H0, [%2]\n" > -" strexd %0, %3, %H3, [%2]\n" > -" teq %0, #0\n" > -" bne 1b" > - : "=&r" (tmp), "=Qo" (v->counter) > - : "r" (&v->counter), "r" (i) > - : "cc"); > -} > #endif > > -#define ATOMIC64_OP(op, op1, op2) \ > -static inline void atomic64_##op(long long i, atomic64_t *v) \ > +#undef __ATOMIC64_READ > + > +#define __ATOMIC64_OP(op, suffix, op1, op2, post_op, extable) \ > +static inline void atomic64_##op##suffix(long long i, atomic64##suffix##_t *v) \ > { \ > long long result; \ > unsigned long tmp; \ > @@ -335,17 +448,31 @@ static inline void atomic64_##op(long long i, atomic64_t *v) \ > "1: ldrexd %0, %H0, [%3]\n" \ > " " #op1 " %Q0, %Q0, %Q4\n" \ > " " #op2 " %R0, %R0, %R4\n" \ > + post_op \ > " strexd %1, %0, %H0, [%3]\n" \ > " teq %1, #0\n" \ > -" bne 1b" \ > +" bne 1b\n" \ > + extable \ > : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) \ > : "r" (&v->counter), "r" (i) \ > : "cc"); \ > -} \ > +} > > -#define ATOMIC64_OP_RETURN(op, op1, op2) \ > +#define ATOMIC64_OP(op, op1, op2) \ > + __ATOMIC64_OP(op, _wrap, op1, op2, , ) \ > + __ATOMIC64_OP(op, , op1, op2##s, __OVERFLOW_POST, __OVERFLOW_EXTABLE) > + > +#undef __OVERFLOW_POST_RETURN > +#define __OVERFLOW_POST_RETURN \ > + "bvc 3f\n" \ > + "mov %0, %1\n" \ > + "mov %H0, %H1\n" \ > + "2: "HARDENED_ATOMIC_INSN"\n" \ > + "3:\n" > + > +#define __ATOMIC64_OP_RETURN(op, suffix, op1, op2, post_op, extable) \ > static inline long long \ > -atomic64_##op##_return_relaxed(long long i, atomic64_t *v) \ > +atomic64_##op##_return##suffix##_relaxed(long long i, atomic64##suffix##_t *v) \ > { \ > long long result; \ > unsigned long tmp; \ > @@ -356,9 +483,11 @@ atomic64_##op##_return_relaxed(long long i, atomic64_t *v) \ > "1: ldrexd %0, %H0, [%3]\n" \ > " " #op1 " %Q0, %Q0, %Q4\n" \ > " " #op2 " %R0, %R0, %R4\n" \ > + post_op \ > " strexd %1, %0, %H0, [%3]\n" \ > " teq %1, #0\n" \ > -" bne 1b" \ > +" bne 1b\n" \ > + extable \ > : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) \ > : "r" (&v->counter), "r" (i) \ > : "cc"); \ > @@ -366,6 +495,11 @@ atomic64_##op##_return_relaxed(long long i, atomic64_t *v) \ > return result; \ > } > > +#define ATOMIC64_OP_RETURN(op, op1, op2) \ > + __ATOMIC64_OP_RETURN(op, _wrap, op1, op2, , ) \ > + __ATOMIC64_OP_RETURN(op, , op1, op2##s, __OVERFLOW_POST_RETURN, \ > + __OVERFLOW_EXTABLE) > + > #define ATOMIC64_FETCH_OP(op, op1, op2) \ > static inline long long \ > atomic64_fetch_##op##_relaxed(long long i, atomic64_t *v) \ > @@ -422,70 +556,98 @@ ATOMIC64_OPS(xor, eor, eor) > #undef ATOMIC64_OPS > #undef ATOMIC64_FETCH_OP > #undef ATOMIC64_OP_RETURN > +#undef __ATOMIC64_OP_RETURN > #undef ATOMIC64_OP > - > -static inline long long > -atomic64_cmpxchg_relaxed(atomic64_t *ptr, long long old, long long new) > -{ > - long long oldval; > - unsigned long res; > - > - prefetchw(&ptr->counter); > - > - do { > - __asm__ __volatile__("@ atomic64_cmpxchg\n" > - "ldrexd %1, %H1, [%3]\n" > - "mov %0, #0\n" > - "teq %1, %4\n" > - "teqeq %H1, %H4\n" > - "strexdeq %0, %5, %H5, [%3]" > - : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) > - : "r" (&ptr->counter), "r" (old), "r" (new) > - : "cc"); > - } while (res); > - > - return oldval; > +#undef __ATOMIC64_OP > +#undef __OVERFLOW_EXTABLE > +#undef __OVERFLOW_POST_RETURN > +#undef __OVERFLOW_RETURN > + > +#define __ATOMIC64_CMPXCHG_RELAXED(suffix) \ > +static inline long long atomic64_cmpxchg##suffix##_relaxed( \ > + atomic64##suffix##_t *ptr, long long old, long long new) \ > +{ \ > + long long oldval; \ > + unsigned long res; \ > + \ > + prefetchw(&ptr->counter); \ > + \ > + do { \ > + __asm__ __volatile__("@ atomic64_cmpxchg" #suffix "\n" \ > + "ldrexd %1, %H1, [%3]\n" \ > + "mov %0, #0\n" \ > + "teq %1, %4\n" \ > + "teqeq %H1, %H4\n" \ > + "strexdeq %0, %5, %H5, [%3]" \ > + : "=&r" (res), "=&r" (oldval), "+Qo" (ptr->counter) \ > + : "r" (&ptr->counter), "r" (old), "r" (new) \ > + : "cc"); \ > + } while (res); \ > + \ > + return oldval; \ > } > -#define atomic64_cmpxchg_relaxed atomic64_cmpxchg_relaxed > > -static inline long long atomic64_xchg_relaxed(atomic64_t *ptr, long long new) > -{ > - long long result; > - unsigned long tmp; > - > - prefetchw(&ptr->counter); > +__ATOMIC64_CMPXCHG_RELAXED() > +__ATOMIC64_CMPXCHG_RELAXED(_wrap) > +#define atomic64_cmpxchg_relaxed atomic64_cmpxchg_relaxed > > - __asm__ __volatile__("@ atomic64_xchg\n" > -"1: ldrexd %0, %H0, [%3]\n" > -" strexd %1, %4, %H4, [%3]\n" > -" teq %1, #0\n" > -" bne 1b" > - : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter) > - : "r" (&ptr->counter), "r" (new) > - : "cc"); > +#undef __ATOMIC64_CMPXCHG_RELAXED > > - return result; > +#define __ATOMIC64_XCHG_RELAXED(suffix) \ > +static inline long long atomic64_xchg##suffix##_relaxed( \ > + atomic64##suffix##_t *ptr, long long new) \ > +{ \ > + long long result; \ > + unsigned long tmp; \ > + \ > + prefetchw(&ptr->counter); \ > + \ > + __asm__ __volatile__("@ atomic64_xchg" #suffix "\n" \ > +"1: ldrexd %0, %H0, [%3]\n" \ > +" strexd %1, %4, %H4, [%3]\n" \ > +" teq %1, #0\n" \ > +" bne 1b" \ > + : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter) \ > + : "r" (&ptr->counter), "r" (new) \ > + : "cc"); \ > + \ > + return result; \ > } > + > +__ATOMIC64_XCHG_RELAXED() > +__ATOMIC64_XCHG_RELAXED(_wrap) > #define atomic64_xchg_relaxed atomic64_xchg_relaxed > > +#undef __ATOMIC64_XCHG_RELAXED > + > static inline long long atomic64_dec_if_positive(atomic64_t *v) > { > long long result; > - unsigned long tmp; > + u64 tmp; > > smp_mb(); > prefetchw(&v->counter); > > __asm__ __volatile__("@ atomic64_dec_if_positive\n" > -"1: ldrexd %0, %H0, [%3]\n" > -" subs %Q0, %Q0, #1\n" > -" sbc %R0, %R0, #0\n" > +"1: ldrexd %1, %H1, [%3]\n" > +" subs %Q0, %Q1, #1\n" > +" sbcs %R0, %R1, #0\n" > +#ifdef CONFIG_HARDENED_ATOMIC > +" bvc 3f\n" > +" mov %Q0, %Q1\n" > +" mov %R0, %R1\n" > +"2: "HARDENED_ATOMIC_INSN"\n" > +"3:\n" > +#endif > " teq %R0, #0\n" > -" bmi 2f\n" > +" bmi 4f\n" > " strexd %1, %0, %H0, [%3]\n" > " teq %1, #0\n" > " bne 1b\n" > -"2:" > +"4:\n" > +#ifdef CONFIG_HARDENED_ATOMIC > + _ASM_EXTABLE(2b, 4b) > +#endif > : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter) > : "r" (&v->counter) > : "cc"); > @@ -509,13 +671,21 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u) > " teq %0, %5\n" > " teqeq %H0, %H5\n" > " moveq %1, #0\n" > -" beq 2f\n" > +" beq 4f\n" > " adds %Q0, %Q0, %Q6\n" > -" adc %R0, %R0, %R6\n" > +" adcs %R0, %R0, %R6\n" > +#ifdef CONFIG_HARDENED_ATOMIC > +" bvc 3f\n" > +"2: "HARDENED_ATOMIC_INSN"\n" > +"3:\n" > +#endif > " strexd %2, %0, %H0, [%4]\n" > " teq %2, #0\n" > " bne 1b\n" > -"2:" > +"4:\n" > +#ifdef CONFIG_HARDENED_ATOMIC > + _ASM_EXTABLE(2b, 4b) > +#endif > : "=&r" (val), "+r" (ret), "=&r" (tmp), "+Qo" (v->counter) > : "r" (&v->counter), "r" (u), "r" (a) > : "cc"); > @@ -529,6 +699,7 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u) > #define atomic64_add_negative(a, v) (atomic64_add_return((a), (v)) < 0) > #define atomic64_inc(v) atomic64_add(1LL, (v)) > #define atomic64_inc_return_relaxed(v) atomic64_add_return_relaxed(1LL, (v)) > +#define atomic64_inc_return_wrap_relaxed(v) atomic64_add_return_wrap_relaxed(1LL, v) > #define atomic64_inc_and_test(v) (atomic64_inc_return(v) == 0) > #define atomic64_sub_and_test(a, v) (atomic64_sub_return((a), (v)) == 0) > #define atomic64_dec(v) atomic64_sub(1LL, (v)) > @@ -536,6 +707,9 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u) > #define atomic64_dec_and_test(v) (atomic64_dec_return((v)) == 0) > #define atomic64_inc_not_zero(v) atomic64_add_unless((v), 1LL, 0LL) > > +#define atomic64_inc_wrap(v) atomic64_add_wrap(1LL, v) > +#define atomic64_dec_wrap(v) atomic64_sub_wrap(1LL, v) > + > #endif /* !CONFIG_GENERIC_ATOMIC64 */ > #endif > #endif > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c > index 3a2e678..ce8ee00 100644 > --- a/arch/arm/mm/fault.c > +++ b/arch/arm/mm/fault.c > @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) > const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); > struct siginfo info; > > +#ifdef CONFIG_HARDENED_ATOMIC > + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { > + unsigned long pc = instruction_pointer(regs); > + unsigned int bkpt; > + > + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && > + cpu_to_le32(bkpt) == 0xe12f1073) { > + current->thread.error_code = ifsr; > + current->thread.trap_no = 0; > + hardened_atomic_overflow(regs); > + fixup_exception(regs); > + return; > + } > + } > +#endif > if (!inf->fn(addr, ifsr | FSR_LNX_PF, regs)) > return; > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-25 9:18 ` AKASHI Takahiro @ 2016-10-25 15:02 ` Colin Vidal 2016-10-26 7:24 ` AKASHI Takahiro 0 siblings, 1 reply; 30+ messages in thread From: Colin Vidal @ 2016-10-25 15:02 UTC (permalink / raw) To: AKASHI Takahiro Cc: kernel-hardening, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand 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@cvidal.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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-25 15:02 ` Colin Vidal @ 2016-10-26 7:24 ` AKASHI Takahiro 2016-10-26 8:20 ` Colin Vidal 0 siblings, 1 reply; 30+ messages in thread From: AKASHI Takahiro @ 2016-10-26 7:24 UTC (permalink / raw) To: Colin Vidal Cc: kernel-hardening, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand On Tue, Oct 25, 2016 at 05:02:47PM +0200, Colin Vidal wrote: > 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@cvidal.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. # I know that this is not a "technical" issue. My point is that _wrap would be the last suffix of the names of all the functions including _relaxed variants for consistency. Say, Elena's atomic-long.h defines: ===8<=== #define ATOMIC_LONG_ADD_SUB_OP(op, mo, suffix) \ static inline long \ atomic_long_##op##_return##mo##suffix(long i, atomic_long##suffix##_t *l)\ { \ ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\ \ return (long)ATOMIC_LONG_PFX(_##op##_return##mo##suffix)(i, v);\ } ATOMIC_LONG_ADD_SUB_OP(add,,) ATOMIC_LONG_ADD_SUB_OP(add, _relaxed,) ... #ifdef CONFIG_HARDENED_ATOMIC ATOMIC_LONG_ADD_SUB_OP(add,,_wrap) ... ===>8=== It would naturally lead to "atomic_long_add_relaxed_wrap" although this function is not actually defined here. > Am I wrong? No, I'm not saying that you are wrong. It's a matter of the naming scheme. We need some consensus. Thanks, -Takahiro AKASHI > Thanks! > > Colin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-26 7:24 ` AKASHI Takahiro @ 2016-10-26 8:20 ` Colin Vidal 2016-10-27 11:08 ` Mark Rutland 0 siblings, 1 reply; 30+ messages in thread From: Colin Vidal @ 2016-10-26 8:20 UTC (permalink / raw) To: AKASHI Takahiro Cc: kernel-hardening, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand Hi Takahiro, > > > > +#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. > > # I know that this is not a "technical" issue. Oh ok, sorry, I misunderstood and I was confused. > > My point is that _wrap would be the last suffix of the names of all > the functions including _relaxed variants for consistency. > > Say, Elena's atomic-long.h defines: > ===8<=== > #define ATOMIC_LONG_ADD_SUB_OP(op, mo, suffix) \ > static inline long \ > atomic_long_##op##_return##mo##suffix(long i, atomic_long##suffix##_t *l)\ > { \ > ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\ > \ > return (long)ATOMIC_LONG_PFX(_##op##_return##mo##suffix)(i, v);\ > } > ATOMIC_LONG_ADD_SUB_OP(add,,) > ATOMIC_LONG_ADD_SUB_OP(add, _relaxed,) > ... > #ifdef CONFIG_HARDENED_ATOMIC > ATOMIC_LONG_ADD_SUB_OP(add,,_wrap) > ... > ===>8=== > > It would naturally lead to "atomic_long_add_relaxed_wrap" although > this function is not actually defined here. > I understand your point, and the need of consistency. "_wrap" is appended to any "user" function of the atomic system, and it is mandatory to have a consistant name for it, I fully agree. However, in the internal implementation of atomic, "_relaxed" is appended to any function that will be bounded by memory barriers. Therefore, a name like "atomic_add_return_wrap_relaxed" makes it easy to see that this function will be embedded in another one. On the other hand "atomic_add_return_relaxed_wrap" is less clear, since it suggest that is a "user" function. I would involve a kind on inconsistency on the naming of relaxed functions. That said, I really don't know which is the "best" naming... :-) Thanks! Colin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-26 8:20 ` Colin Vidal @ 2016-10-27 11:08 ` Mark Rutland 2016-10-27 21:37 ` Kees Cook 0 siblings, 1 reply; 30+ messages in thread From: Mark Rutland @ 2016-10-27 11:08 UTC (permalink / raw) To: kernel-hardening Cc: AKASHI Takahiro, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand On Wed, Oct 26, 2016 at 10:20:16AM +0200, Colin Vidal wrote: > > My point is that _wrap would be the last suffix of the names of all > > the functions including _relaxed variants for consistency. > > > > Say, Elena's atomic-long.h defines: > > ===8<=== > > #define ATOMIC_LONG_ADD_SUB_OP(op, mo, suffix) \ > > static inline long \ > > atomic_long_##op##_return##mo##suffix(long i, atomic_long##suffix##_t *l)\ > > { \ > > ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\ > > \ > > return (long)ATOMIC_LONG_PFX(_##op##_return##mo##suffix)(i, v);\ > > } > > ATOMIC_LONG_ADD_SUB_OP(add,,) > > ATOMIC_LONG_ADD_SUB_OP(add, _relaxed,) > > ... > > #ifdef CONFIG_HARDENED_ATOMIC > > ATOMIC_LONG_ADD_SUB_OP(add,,_wrap) > > ... > > ===>8=== > > > > It would naturally lead to "atomic_long_add_relaxed_wrap" although > > this function is not actually defined here. > > > > I understand your point, and the need of consistency. "_wrap" is > appended to any "user" function of the atomic system, and it is > mandatory to have a consistant name for it, I fully agree. > > However, in the internal implementation of atomic, "_relaxed" is > appended to any function that will be bounded by memory barriers. > Therefore, a name like "atomic_add_return_wrap_relaxed" makes it easy > to see that this function will be embedded in another one. On the other > hand "atomic_add_return_relaxed_wrap" is less clear, since it suggest > that is a "user" function. I would involve a kind on inconsistency on > the naming of relaxed functions. > > That said, I really don't know which is the "best" naming... :-) Given it looks like there's at least one necessary round of bikeshedding, it would be best to discuss this with the usual atomic maintainers included, so as to avoid several more rounds over subsequent postings. i.e. [mark@leverpostej:~/src/linux]% ./scripts/get_maintainer.pl -f \ include/asm-generic/atomic.h \ include/asm-generic/atomic-long.h \ include/linux/atomic.h Arnd Bergmann <arnd@arndb.de> (maintainer:GENERIC INCLUDE/ASM HEADER FILES) Ingo Molnar <mingo@kernel.org> (commit_signer:8/11=73%) Peter Zijlstra <peterz@infradead.org> (commit_signer:6/11=55%,authored:5/11=45%,added_lines:491/671=73%,removed_lines:186/225=83%) Frederic Weisbecker <fweisbec@gmail.com> (commit_signer:3/11=27%,authored:3/11=27%,added_lines:42/671=6%,removed_lines:21/225=9%) Boqun Feng <boqun.feng@gmail.com> (commit_signer:1/11=9%,authored:1/11=9%) Chris Metcalf <cmetcalf@ezchip.com> (commit_signer:1/11=9%) Davidlohr Bueso <dave@stgolabs.net> (authored:1/11=9%,added_lines:128/671=19%) linux-arch@vger.kernel.org (open list:GENERIC INCLUDE/ASM HEADER FILES) linux-kernel@vger.kernel.org (open list) Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-27 11:08 ` Mark Rutland @ 2016-10-27 21:37 ` Kees Cook 0 siblings, 0 replies; 30+ messages in thread From: Kees Cook @ 2016-10-27 21:37 UTC (permalink / raw) To: Mark Rutland Cc: kernel-hardening, AKASHI Takahiro, Reshetova, Elena, David Windsor, Hans Liljestrand On Thu, Oct 27, 2016 at 4:08 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Wed, Oct 26, 2016 at 10:20:16AM +0200, Colin Vidal wrote: > Given it looks like there's at least one necessary round of bikeshedding, it > would be best to discuss this with the usual atomic maintainers included, so as > to avoid several more rounds over subsequent postings. i.e. > > [mark@leverpostej:~/src/linux]% ./scripts/get_maintainer.pl -f \ > include/asm-generic/atomic.h \ > include/asm-generic/atomic-long.h \ > include/linux/atomic.h > Arnd Bergmann <arnd@arndb.de> (maintainer:GENERIC INCLUDE/ASM HEADER FILES) > Ingo Molnar <mingo@kernel.org> (commit_signer:8/11=73%) > Peter Zijlstra <peterz@infradead.org> (commit_signer:6/11=55%,authored:5/11=45%,added_lines:491/671=73%,removed_lines:186/225=83%) > Frederic Weisbecker <fweisbec@gmail.com> (commit_signer:3/11=27%,authored:3/11=27%,added_lines:42/671=6%,removed_lines:21/225=9%) > Boqun Feng <boqun.feng@gmail.com> (commit_signer:1/11=9%,authored:1/11=9%) > Chris Metcalf <cmetcalf@ezchip.com> (commit_signer:1/11=9%) > Davidlohr Bueso <dave@stgolabs.net> (authored:1/11=9%,added_lines:128/671=19%) > linux-arch@vger.kernel.org (open list:GENERIC INCLUDE/ASM HEADER FILES) > linux-kernel@vger.kernel.org (open list) Yeah, I'll be bringing the hardened atomic patch up during kernel summit, but I think the RFC has gotten to the point where we need to loop in more maintainers. Thanks! -Kees -- Kees Cook Nexus Security ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-18 14:59 ` [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC Colin Vidal 2016-10-18 21:29 ` [kernel-hardening] " Kees Cook 2016-10-25 9:18 ` AKASHI Takahiro @ 2016-10-27 13:24 ` Mark Rutland 2016-10-28 5:18 ` AKASHI Takahiro 2016-10-28 8:33 ` Colin Vidal 2 siblings, 2 replies; 30+ messages in thread From: Mark Rutland @ 2016-10-27 13:24 UTC (permalink / raw) To: kernel-hardening Cc: Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand, Colin Vidal Hi, 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. I have some comments below, but for real review this needs to go via the linux-arm-kernel list. > 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" Please put the immediate in a #define somewhere. What about thumb2 kernels? > +#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 > + All this should live close to the assembly using it, to make it possible to follow. This may also not be the best way of structuring this code. The additional indirection of passing this in at a high level makes it hard to read and potentially fragile. For single instructions it was ok, but I'm not so sure that it's ok for larger sequences like this. > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c > index 3a2e678..ce8ee00 100644 > --- a/arch/arm/mm/fault.c > +++ b/arch/arm/mm/fault.c > @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) > const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); > struct siginfo info; > > +#ifdef CONFIG_HARDENED_ATOMIC > + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { You'll need to justify why this isn't in the ifsr_info table. It has the same basic shape as the usual set of handlers. I note that we don't seem to use SW breakpoints at all currently, and I suspect there's a reason for that which we need to consider. Also, if this *must* live here, please make it a static inline with an empty stub, rather than an ifdef'd block. > + unsigned long pc = instruction_pointer(regs); > + unsigned int bkpt; > + > + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && > + cpu_to_le32(bkpt) == 0xe12f1073) { This appears to be the A1 encoding from the ARM ARM. What about the T1 encoding, i.e. thumb? Regardless, *please* de-magic the number using a #define. Also, this should be le32_to_cpu -- in the end we're treating the coverted value as cpu-native. The variable itself should be a __le32. Thanks, Mark. > + current->thread.error_code = ifsr; > + current->thread.trap_no = 0; > + hardened_atomic_overflow(regs); > + fixup_exception(regs); > + return; > + } > + } > +#endif > if (!inf->fn(addr, ifsr | FSR_LNX_PF, regs)) > return; > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-27 13:24 ` [kernel-hardening] " Mark Rutland @ 2016-10-28 5:18 ` AKASHI Takahiro 2016-10-28 8:33 ` Colin Vidal 1 sibling, 0 replies; 30+ messages in thread From: AKASHI Takahiro @ 2016-10-28 5:18 UTC (permalink / raw) To: Mark Rutland Cc: kernel-hardening, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand, Colin Vidal On Thu, Oct 27, 2016 at 02:24:36PM +0100, Mark Rutland wrote: > Hi, > > 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. > > I have some comments below, but for real review this needs to go via the > linux-arm-kernel list. Yeah, definitely, but > > 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" > > Please put the immediate in a #define somewhere. > > What about thumb2 kernels? > > > +#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 > > + > > All this should live close to the assembly using it, to make it possible > to follow. > > This may also not be the best way of structuring this code. The > additional indirection of passing this in at a high level makes it hard > to read and potentially fragile. For single instructions it was ok, but > I'm not so sure that it's ok for larger sequences like this. I did the similar thing for arm64 port (ll/sc version) as the current macros are already complicated and I have no other better idea to generate definitions for protected and _wrap versions equally. See below. What are different from Colin's arm port are: * control __HARDENED_ATOMIC_CHECK/__CL* directly instead of passing them as an argument * modify regs->pc directly in a handler to skip "brk" (not shown in this hunk) instead of using _ASM_EXTABLE (& fixup_exception()) Anyway, I'm putting off posting my arm64 port while some discussions are going on against Elena's x86 patch. Thanks, -Takahiro AKASHI ===8<=== #define ATOMIC_OP(op, asm_op, wrap, cl) \ ... #define ATOMIC_OP_RETURN(name, mb, acq, rel, op, asm_op, wrap, cl) \ __LL_SC_INLINE int \ __LL_SC_PREFIX(atomic_##op##_return##wrap##name(int i, atomic##wrap##_t *v))\ { \ unsigned long tmp; \ int result; \ \ asm volatile("// atomic_" #op "_return" #name "\n" \ " prfm pstl1strm, %2\n" \ "1: ld" #acq "xr %w0, %2\n" \ " " #asm_op " %w0, %w0, %w3\n" \ __HARDENED_ATOMIC_CHECK \ " st" #rel "xr %w1, %w0, %2\n" \ " cbnz %w1, 1b\n" \ " " #mb "\n" \ "4:" \ : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \ : "Ir" (i) \ : cl); \ \ return result; \ } \ __LL_SC_EXPORT(atomic_##op##_return##wrap##name); #define ATOMIC_FETCH_OP(name, mb, acq, rel, op, asm_op, wrap, cl) \ ... #define ATOMIC_OPS(...) \ ATOMIC_OP(__VA_ARGS__, __CL) \ ATOMIC_OP_RETURN( , dmb ish, , l, __VA_ARGS__, __CL_MEM)\ ATOMIC_OP_RETURN(_relaxed, , , , __VA_ARGS__, __CL) \ ATOMIC_OP_RETURN(_acquire, , a, , __VA_ARGS__, __CL_MEM)\ ATOMIC_OP_RETURN(_release, , , l, __VA_ARGS__, __CL_MEM)\ ATOMIC_FETCH_OP ( , dmb ish, , l, __VA_ARGS__, __CL_MEM)\ ATOMIC_FETCH_OP (_relaxed, , , , __VA_ARGS__, __CL) \ ATOMIC_FETCH_OP (_acquire, , a, , __VA_ARGS__, __CL_MEM)\ ATOMIC_FETCH_OP (_release, , , l, __VA_ARGS__, __CL_MEM) #ifdef CONFIG_HARDENED_ATOMIC #define __HARDENED_ATOMIC_CHECK \ " bvc 3f\n" \ "2: brk " __stringify(BUG_ATOMIC_OVERFLOW_BRK_IMM) "\n" \ " b 4f\n" \ "3:" #define __CL "cc" #define __CL_MEM "cc", "memory" ATOMIC_OPS(add, adds, ) ATOMIC_OPS(sub, subs, ) #else #define __HARDENED_ATOMIC_CHECK #define __CL #define __CL_MEM ATOMIC_OPS(add, add, ) ATOMIC_OPS(sub, sub, ) #endif #undef __HARDENED_ATOMIC_CHECK #define __HARDENED_ATOMIC_CHECK #undef __CL #undef __CL_MEM #define __CL #define __CL_MEM "memory" ATOMIC_OPS(add, add, _wrap) ATOMIC_OPS(sub, sub, _wrap) ===>8=== > > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c > > index 3a2e678..ce8ee00 100644 > > --- a/arch/arm/mm/fault.c > > +++ b/arch/arm/mm/fault.c > > @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) > > const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); > > struct siginfo info; > > > > +#ifdef CONFIG_HARDENED_ATOMIC > > + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { > > You'll need to justify why this isn't in the ifsr_info table. It has the > same basic shape as the usual set of handlers. > > I note that we don't seem to use SW breakpoints at all currently, and I > suspect there's a reason for that which we need to consider. > > Also, if this *must* live here, please make it a static inline with an > empty stub, rather than an ifdef'd block. > > > + unsigned long pc = instruction_pointer(regs); > > + unsigned int bkpt; > > + > > + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && > > + cpu_to_le32(bkpt) == 0xe12f1073) { > > This appears to be the A1 encoding from the ARM ARM. What about the T1 > encoding, i.e. thumb? > > Regardless, *please* de-magic the number using a #define. > > Also, this should be le32_to_cpu -- in the end we're treating the > coverted value as cpu-native. The variable itself should be a __le32. > > Thanks, > Mark. > > > + current->thread.error_code = ifsr; > > + current->thread.trap_no = 0; > > + hardened_atomic_overflow(regs); > > + fixup_exception(regs); > > + return; > > + } > > + } > > +#endif > > if (!inf->fn(addr, ifsr | FSR_LNX_PF, regs)) > > return; > > > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-27 13:24 ` [kernel-hardening] " Mark Rutland 2016-10-28 5:18 ` AKASHI Takahiro @ 2016-10-28 8:33 ` Colin Vidal 2016-10-28 10:20 ` Mark Rutland 1 sibling, 1 reply; 30+ messages in thread From: Colin Vidal @ 2016-10-28 8:33 UTC (permalink / raw) To: Mark Rutland, kernel-hardening Cc: Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand Hi Mark, On Thu, 2016-10-27 at 14:24 +0100, Mark Rutland wrote: > Hi, > > 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. > > I have some comments below, but for real review this needs to go via the > linux-arm-kernel list. Thanks a lot for that! Yep, I will CC linux-arm-kernel, linux-kernel and maintainers of atomic/arm for the next RFC. I take info account your remarks for the next RFC, but I have some questions for some of them bellow. > > > > 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" > > Please put the immediate in a #define somewhere. I an not sure to get what do you mean here. 0xf103 should be in a #define? (as for the A1 encoded version, for sure). > > What about thumb2 kernels? > > > > > +#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 > > + > All this should live close to the assembly using it, to make it possible > to follow. > > This may also not be the best way of structuring this code. The > additional indirection of passing this in at a high level makes it hard > to read and potentially fragile. For single instructions it was ok, but > I'm not so sure that it's ok for larger sequences like this. > I agree that is quite difficult to follow, but as Takahiro said, the current macro are already complex. Still, I will try to put those definitions closer of the code using them (for instance, just before the macro expansions?), but I am not sure that would change anything: the code using it is broke up in different area of the file anyways. Besides, I really would avoid using an extable entry, since the fixup address is never used, I am not sure it is mandatory (and it seems Takahiro does not using it, too). > > > > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c > > index 3a2e678..ce8ee00 100644 > > --- a/arch/arm/mm/fault.c > > +++ b/arch/arm/mm/fault.c > > @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) > > const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); > > struct siginfo info; > > > > +#ifdef CONFIG_HARDENED_ATOMIC > > + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { > > You'll need to justify why this isn't in the ifsr_info table. It has the > same basic shape as the usual set of handlers. > > I note that we don't seem to use SW breakpoints at all currently, and I > suspect there's a reason for that which we need to consider. > > Also, if this *must* live here, please make it a static inline with an > empty stub, rather than an ifdef'd block. > > > > > + unsigned long pc = instruction_pointer(regs); > > + unsigned int bkpt; > > + > > + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && > > + cpu_to_le32(bkpt) == 0xe12f1073) { > > This appears to be the A1 encoding from the ARM ARM. What about the T1 > encoding, i.e. thumb? > > Regardless, *please* de-magic the number using a #define. > > Also, this should be le32_to_cpu -- in the end we're treating the > coverted value as cpu-native. The variable itself should be a __le32. > I must confess that I am still very confused about the code in fault.c (and the ifsr_info table). I will take time to try to understand a little bit more that code before submitting a new RFC. Still, it you have some pointers/documentations about it, I would be grateful. Thanks! Colin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-28 8:33 ` Colin Vidal @ 2016-10-28 10:20 ` Mark Rutland 2016-10-28 10:59 ` David Windsor 0 siblings, 1 reply; 30+ messages in thread From: Mark Rutland @ 2016-10-28 10:20 UTC (permalink / raw) To: Colin Vidal Cc: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand On Fri, Oct 28, 2016 at 10:33:15AM +0200, Colin Vidal wrote: > On Thu, 2016-10-27 at 14:24 +0100, Mark Rutland wrote: > > On Tue, Oct 18, 2016 at 04:59:21PM +0200, Colin Vidal wrote: > > > +#define HARDENED_ATOMIC_INSN "bkpt 0xf103" > > > > Please put the immediate in a #define somewhere. > > I an not sure to get what do you mean here. 0xf103 should be in a > #define? (as for the A1 encoded version, for sure). I mean have something like: #ifdef CONFIG_THUMB2_KERNEL #define ATOMIC_BKPT_IMM 0xf1 #else #define ATOMIC_BKPT_IMM 0xf103 #endif With code having something like: HARDENED_ATOMIC_INSN "bkpt" #ATOMIC_BKPT_IMM ... or passing that in via an %i template to the asm. That way, we can also build the encoding for the exception path from the same immediate. That will keep the two in sync, making the relationship between the two obvious. e.g. first add something like arm_get_bkpt(imm) to <asm/insn.h>. > > > +#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 > > > + > > All this should live close to the assembly using it, to make it possible > > to follow. > > > > This may also not be the best way of structuring this code. The > > additional indirection of passing this in at a high level makes it hard > > to read and potentially fragile. For single instructions it was ok, but > > I'm not so sure that it's ok for larger sequences like this. > > I agree that is quite difficult to follow, but as Takahiro said, the > current macro are already complex. Still, I will try to put those > definitions closer of the code using them (for instance, just before > the macro expansions?), but I am not sure that would change anything: > the code using it is broke up in different area of the file anyways. I'm not sure that's true. IIRC each of the cases above is only used by one template case, and we could instead fold that into the template. For example, if we had something like the ALTERNATIVE macros that worked on some boolean passed in, so you could have: #define __foo_asm_template(__bool, params ...) \ asm( \ "insn reg, whatever" \ TEMPLATE(__bool, "code_if_bool", "code if_not_bool") \ "more insns..." \ clobbers_etc) That way all the code is in one place, and easier to follow, and we can generate both the instrumented and non-instrumented variants from the same template. > Besides, I really would avoid using an extable entry, since the fixup > address is never used, I am not sure it is mandatory (and it seems > Takahiro does not using it, too). >From the grsecurity blog post [1], per the comments in the ARM detection logic, this is used to continue after warning (and not incrementing) in the overflow case. What do we do differently in the overflow case? > > > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c > > > index 3a2e678..ce8ee00 100644 > > > --- a/arch/arm/mm/fault.c > > > +++ b/arch/arm/mm/fault.c > > > @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) > > > const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); > > > struct siginfo info; > > > > > > +#ifdef CONFIG_HARDENED_ATOMIC > > > + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { > > > > You'll need to justify why this isn't in the ifsr_info table. It has the > > same basic shape as the usual set of handlers. > > > > I note that we don't seem to use SW breakpoints at all currently, and I > > suspect there's a reason for that which we need to consider. > > > > Also, if this *must* live here, please make it a static inline with an > > empty stub, rather than an ifdef'd block. > > > > > > > > + unsigned long pc = instruction_pointer(regs); > > > + unsigned int bkpt; > > > + > > > + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && > > > + cpu_to_le32(bkpt) == 0xe12f1073) { > > > > This appears to be the A1 encoding from the ARM ARM. What about the T1 > > encoding, i.e. thumb? > > > > Regardless, *please* de-magic the number using a #define. > > > > Also, this should be le32_to_cpu -- in the end we're treating the > > coverted value as cpu-native. The variable itself should be a __le32. > > I must confess that I am still very confused about the code in fault.c > (and the ifsr_info table). I will take time to try to understand a > little bit more that code before submitting a new RFC. Still, it you > have some pointers/documentations about it, I would be grateful. Unfortuantely, I'm not aware of any documentation for this code. The ifsr_info table is just a set of handlers indexed by the ifsr value. Ignoring the conflict with the HW breakpoint handler, you'd be able to install this in the FAULT_CODE_DEBUG slot of ifsr_info. Thanks, Mark. [1] https://forums.grsecurity.net/viewtopic.php?f=7&t=4173 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC 2016-10-28 10:20 ` Mark Rutland @ 2016-10-28 10:59 ` David Windsor 0 siblings, 0 replies; 30+ messages in thread From: David Windsor @ 2016-10-28 10:59 UTC (permalink / raw) To: kernel-hardening Cc: Colin Vidal, Reshetova, Elena, AKASHI Takahiro, Kees Cook, Hans Liljestrand On Fri, Oct 28, 2016 at 6:20 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Oct 28, 2016 at 10:33:15AM +0200, Colin Vidal wrote: >> On Thu, 2016-10-27 at 14:24 +0100, Mark Rutland wrote: >> > On Tue, Oct 18, 2016 at 04:59:21PM +0200, Colin Vidal wrote: >> > > +#define HARDENED_ATOMIC_INSN "bkpt 0xf103" >> > >> > Please put the immediate in a #define somewhere. >> >> I an not sure to get what do you mean here. 0xf103 should be in a >> #define? (as for the A1 encoded version, for sure). > > I mean have something like: > > #ifdef CONFIG_THUMB2_KERNEL > #define ATOMIC_BKPT_IMM 0xf1 > #else > #define ATOMIC_BKPT_IMM 0xf103 > #endif > > With code having something like: > > HARDENED_ATOMIC_INSN "bkpt" #ATOMIC_BKPT_IMM > > ... or passing that in via an %i template to the asm. > > That way, we can also build the encoding for the exception path from the > same immediate. That will keep the two in sync, making the relationship > between the two obvious. e.g. first add something like arm_get_bkpt(imm) > to <asm/insn.h>. > >> > > +#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 >> > > + >> > All this should live close to the assembly using it, to make it possible >> > to follow. >> > >> > This may also not be the best way of structuring this code. The >> > additional indirection of passing this in at a high level makes it hard >> > to read and potentially fragile. For single instructions it was ok, but >> > I'm not so sure that it's ok for larger sequences like this. >> >> I agree that is quite difficult to follow, but as Takahiro said, the >> current macro are already complex. Still, I will try to put those >> definitions closer of the code using them (for instance, just before >> the macro expansions?), but I am not sure that would change anything: >> the code using it is broke up in different area of the file anyways. > > I'm not sure that's true. IIRC each of the cases above is only used by > one template case, and we could instead fold that into the template. > For example, if we had something like the ALTERNATIVE macros that worked > on some boolean passed in, so you could have: > > #define __foo_asm_template(__bool, params ...) \ > asm( \ > "insn reg, whatever" \ > TEMPLATE(__bool, "code_if_bool", "code if_not_bool") \ > "more insns..." \ > clobbers_etc) > > That way all the code is in one place, and easier to follow, and we can > generate both the instrumented and non-instrumented variants from the > same template. > >> Besides, I really would avoid using an extable entry, since the fixup >> address is never used, I am not sure it is mandatory (and it seems >> Takahiro does not using it, too). > > From the grsecurity blog post [1], per the comments in the ARM detection > logic, this is used to continue after warning (and not incrementing) in > the overflow case. > > What do we do differently in the overflow case? > In the overflow case, exception/trap code is called, which invokes hardened_atomic_report_overflow(), which then calls BUG(). At least on x86. >> > > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c >> > > index 3a2e678..ce8ee00 100644 >> > > --- a/arch/arm/mm/fault.c >> > > +++ b/arch/arm/mm/fault.c >> > > @@ -580,6 +580,21 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs) >> > > const struct fsr_info *inf = ifsr_info + fsr_fs(ifsr); >> > > struct siginfo info; >> > > >> > > +#ifdef CONFIG_HARDENED_ATOMIC >> > > + if (fsr_fs(ifsr) == FAULT_CODE_DEBUG) { >> > >> > You'll need to justify why this isn't in the ifsr_info table. It has the >> > same basic shape as the usual set of handlers. >> > >> > I note that we don't seem to use SW breakpoints at all currently, and I >> > suspect there's a reason for that which we need to consider. >> > >> > Also, if this *must* live here, please make it a static inline with an >> > empty stub, rather than an ifdef'd block. >> > >> > > >> > > + unsigned long pc = instruction_pointer(regs); >> > > + unsigned int bkpt; >> > > + >> > > + if (!probe_kernel_address((const unsigned int *)pc, bkpt) && >> > > + cpu_to_le32(bkpt) == 0xe12f1073) { >> > >> > This appears to be the A1 encoding from the ARM ARM. What about the T1 >> > encoding, i.e. thumb? >> > >> > Regardless, *please* de-magic the number using a #define. >> > >> > Also, this should be le32_to_cpu -- in the end we're treating the >> > coverted value as cpu-native. The variable itself should be a __le32. >> >> I must confess that I am still very confused about the code in fault.c >> (and the ifsr_info table). I will take time to try to understand a >> little bit more that code before submitting a new RFC. Still, it you >> have some pointers/documentations about it, I would be grateful. > > Unfortuantely, I'm not aware of any documentation for this code. The > ifsr_info table is just a set of handlers indexed by the ifsr value. > > Ignoring the conflict with the HW breakpoint handler, you'd be able to > install this in the FAULT_CODE_DEBUG slot of ifsr_info. > > Thanks, > Mark. > > [1] https://forums.grsecurity.net/viewtopic.php?f=7&t=4173 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [kernel-hardening] Re: [RFC 0/2] arm: implementation of HARDENED_ATOMIC 2016-10-18 14:59 [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC Colin Vidal 2016-10-18 14:59 ` [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset Colin Vidal 2016-10-18 14:59 ` [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC Colin Vidal @ 2016-10-21 7:47 ` AKASHI Takahiro 2016-10-27 10:32 ` [kernel-hardening] " Mark Rutland 3 siblings, 0 replies; 30+ messages in thread From: AKASHI Takahiro @ 2016-10-21 7:47 UTC (permalink / raw) To: Colin Vidal Cc: kernel-hardening, Reshetova, Elena, David Windsor, Kees Cook, Hans Liljestrand On Tue, Oct 18, 2016 at 04:59:19PM +0200, Colin Vidal wrote: > Hi, > > This is the first attempt of HARDENED_ATOMIC port to arm arch. > > About the fault handling I have some questions (perhaps some arm > expert are reading?): > > - As the process that made the overflow is killed, the kernel will > not try to go to a fixup address when the exception is raised, > right ? Therefore, is still mandatory to add an entry in the > __extable section? > > - In do_PrefetchAbort, I am unsure the code that follow the call to > hardened_atomic_overflow is needed: the process will be killed > anyways. I don't know. Even in x86 implementation, when an overflow is detected, say, in atomic_add(), the operation is supported to be canceled (ie. by reverting the atomic_t variable to the original value). -Takahiro AKASHI > I take some freedom compared to PaX patch, especially by adding some > macro to expand functions in arm/include/asm/atomic.h. > > The first patch is the modification I have done is generic part to > make it work. > > Otherwise, I've been stuck by ccache. When I modify do_PrefetchAbort > in arm/mm/fault.c, ccache does not detect the update (even if the file > is recompiled by gcc). Therefore, when I boot the new compiled kernel, > the old version of do_PrefechAbort is called. I know do_PrefetchAbort > is somehow special, since called by assembly code, but is still > strange. Someone has already has this issue? The only way to solve it > is to flush the cache... > > Thanks! > > Colin > > Colin Vidal (2): > Reordering / guard definition on atomic_*_wrap function in order to > avoid implicitly defined / redefined error on them, when > CONFIG_HARDENED_ATOMIC is unset. > arm: implementation for HARDENED_ATOMIC > > arch/arm/Kconfig | 1 + > arch/arm/include/asm/atomic.h | 434 ++++++++++++++++++++++++++------------ > arch/arm/mm/fault.c | 15 ++ > include/asm-generic/atomic-long.h | 55 ++--- > include/linux/atomic.h | 55 +++++ > 5 files changed, 405 insertions(+), 155 deletions(-) > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC 2016-10-18 14:59 [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC Colin Vidal ` (2 preceding siblings ...) 2016-10-21 7:47 ` [kernel-hardening] Re: [RFC 0/2] arm: implementation of HARDENED_ATOMIC AKASHI Takahiro @ 2016-10-27 10:32 ` Mark Rutland 2016-10-27 12:45 ` David Windsor 3 siblings, 1 reply; 30+ messages in thread From: Mark Rutland @ 2016-10-27 10:32 UTC (permalink / raw) To: kernel-hardening Cc: Reshetova, Elena, AKASHI Takahiro, David Windsor, Kees Cook, Hans Liljestrand, Colin Vidal Hi, On Tue, Oct 18, 2016 at 04:59:19PM +0200, Colin Vidal wrote: > Hi, > > This is the first attempt of HARDENED_ATOMIC port to arm arch. As a general note, please Cc relevant lists and people, as per get_maintainer.pl. For these patches that should tell you to Cc linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, and a number of people familiar with the atomics. Even if things are far from perfect, and people don't reply (or reply but not too kindly), having them on Cc earlier makes it far more likely that issues are spotted and addressed earlier, minimizes repeatedly discussing the same issues, and also minimizes the potential for future arguments about these things being developed in isolation. Unless you do that, critical review for core code and arch code will come very late, and that could potentially delay this being merged for a very long time, which would be unfortunate. > About the fault handling I have some questions (perhaps some arm > expert are reading?): > > - As the process that made the overflow is killed, the kernel will > not try to go to a fixup address when the exception is raised, > right ? Therefore, is still mandatory to add an entry in the > __extable section? > > - In do_PrefetchAbort, I am unsure the code that follow the call to > hardened_atomic_overflow is needed: the process will be killed > anyways. Unfortunately, I'm only somewhat familiar with the ARM atomics, and I have absolutely no familiarity with the existing PaX patchset. For both of these, some background rationale would be helpful. e.g. what does the fixup entry do? When is it invoked? I'll see what I can reverse-engineer from the patches. > I take some freedom compared to PaX patch, especially by adding some > macro to expand functions in arm/include/asm/atomic.h. > > The first patch is the modification I have done is generic part to > make it work. If you're relying on a prior patch series, please refer to that in the cover, to make it possible for reviewers to find. If you have a public git repo, placing this in a branch (or a tagged commit), and referring to that in the cover messages would make it much easier for people to review and/or test. Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC 2016-10-27 10:32 ` [kernel-hardening] " Mark Rutland @ 2016-10-27 12:45 ` David Windsor 2016-10-27 13:53 ` Mark Rutland 0 siblings, 1 reply; 30+ messages in thread From: David Windsor @ 2016-10-27 12:45 UTC (permalink / raw) To: Mark Rutland Cc: kernel-hardening, Reshetova, Elena, AKASHI Takahiro, Kees Cook, Hans Liljestrand, Colin Vidal Hi Mark, On Thu, Oct 27, 2016 at 6:32 AM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi, > > On Tue, Oct 18, 2016 at 04:59:19PM +0200, Colin Vidal wrote: >> Hi, >> >> This is the first attempt of HARDENED_ATOMIC port to arm arch. > > As a general note, please Cc relevant lists and people, as per > get_maintainer.pl. For these patches that should tell you to Cc > linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, and > a number of people familiar with the atomics. > > Even if things are far from perfect, and people don't reply (or reply > but not too kindly), having them on Cc earlier makes it far more likely > that issues are spotted and addressed earlier, minimizes repeatedly > discussing the same issues, and also minimizes the potential for future > arguments about these things being developed in isolation. > > Unless you do that, critical review for core code and arch code will > come very late, and that could potentially delay this being merged for a > very long time, which would be unfortunate. > >> About the fault handling I have some questions (perhaps some arm >> expert are reading?): >> >> - As the process that made the overflow is killed, the kernel will >> not try to go to a fixup address when the exception is raised, >> right ? Therefore, is still mandatory to add an entry in the >> __extable section? >> >> - In do_PrefetchAbort, I am unsure the code that follow the call to >> hardened_atomic_overflow is needed: the process will be killed >> anyways. > > Unfortunately, I'm only somewhat familiar with the ARM atomics, and I > have absolutely no familiarity with the existing PaX patchset. > > For both of these, some background rationale would be helpful. e.g. what > does the fixup entry do? When is it invoked? > For your reference, documentation on the original PaX protection (known there a PAX_REFCOUNT) can be found here: https://forums.grsecurity.net/viewtopic.php?f=7&t=4173 With respect to documentation, there is a patch in this series that adds Documentation/security/hardened-atomic.txt, which references the above-mentioned forum post. Although, for long-term maintenance, maybe forum posts aren't the most reliable thing in the world... > I'll see what I can reverse-engineer from the patches. > >> I take some freedom compared to PaX patch, especially by adding some >> macro to expand functions in arm/include/asm/atomic.h. >> >> The first patch is the modification I have done is generic part to >> make it work. > > If you're relying on a prior patch series, please refer to that in the > cover, to make it possible for reviewers to find. > > If you have a public git repo, placing this in a branch (or a tagged > commit), and referring to that in the cover messages would make it much > easier for people to review and/or test. > > Thanks, > Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC 2016-10-27 12:45 ` David Windsor @ 2016-10-27 13:53 ` Mark Rutland 2016-10-27 14:10 ` David Windsor 0 siblings, 1 reply; 30+ messages in thread From: Mark Rutland @ 2016-10-27 13:53 UTC (permalink / raw) To: kernel-hardening Cc: Reshetova, Elena, AKASHI Takahiro, Kees Cook, Hans Liljestrand, Colin Vidal Hi, On Thu, Oct 27, 2016 at 08:45:33AM -0400, David Windsor wrote: > On Thu, Oct 27, 2016 at 6:32 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > Unfortunately, I'm only somewhat familiar with the ARM atomics, and I > > have absolutely no familiarity with the existing PaX patchset. > > > > For both of these, some background rationale would be helpful. e.g. what > > does the fixup entry do? When is it invoked? > > For your reference, documentation on the original PaX protection > (known there a PAX_REFCOUNT) can be found here: > https://forums.grsecurity.net/viewtopic.php?f=7&t=4173 Thanks; that's very helpful. For subsequent postings it would be worth referring to this in the cover letter, along with a rough summary. > With respect to documentation, there is a patch in this series that > adds Documentation/security/hardened-atomic.txt, which references the > above-mentioned forum post. Unfortunately, that's not part of *this* series, and the prerequisite series with this was not linked to. I can find that by going through the list, but for the sake of others, having an explicit link to the relevant version of the other series would be more helpful. Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC 2016-10-27 13:53 ` Mark Rutland @ 2016-10-27 14:10 ` David Windsor 0 siblings, 0 replies; 30+ messages in thread From: David Windsor @ 2016-10-27 14:10 UTC (permalink / raw) To: kernel-hardening Cc: Reshetova, Elena, AKASHI Takahiro, Kees Cook, Hans Liljestrand, Colin Vidal On Thu, Oct 27, 2016 at 9:53 AM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi, > > On Thu, Oct 27, 2016 at 08:45:33AM -0400, David Windsor wrote: >> On Thu, Oct 27, 2016 at 6:32 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> > Unfortunately, I'm only somewhat familiar with the ARM atomics, and I >> > have absolutely no familiarity with the existing PaX patchset. >> > >> > For both of these, some background rationale would be helpful. e.g. what >> > does the fixup entry do? When is it invoked? >> >> For your reference, documentation on the original PaX protection >> (known there a PAX_REFCOUNT) can be found here: >> https://forums.grsecurity.net/viewtopic.php?f=7&t=4173 > > Thanks; that's very helpful. For subsequent postings it would be worth > referring to this in the cover letter, along with a rough summary. > >> With respect to documentation, there is a patch in this series that >> adds Documentation/security/hardened-atomic.txt, which references the >> above-mentioned forum post. > > Unfortunately, that's not part of *this* series, and the prerequisite > series with this was not linked to. I can find that by going through the > list, but for the sake of others, having an explicit link to the > relevant version of the other series would be more helpful. > Ah yes, you're right, the HARDENED_ATOMIC documentation patch isn't part of this series at all! My point was, might we want to do something with the original forum post to make it more suitable for "long term storage"? Maybe a forum post is the appropriate venue for a project's technical documentation; I don't feel qualified to make this determination. > Thanks, > Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2016-10-28 10:59 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-18 14:59 [kernel-hardening] [RFC 0/2] arm: implementation of HARDENED_ATOMIC Colin Vidal 2016-10-18 14:59 ` [kernel-hardening] [RFC 1/2] Reordering / guard definition on atomic_*_wrap function in order to avoid implicitly defined / redefined error on them, when CONFIG_HARDENED_ATOMIC is unset Colin Vidal 2016-10-18 16:04 ` Vaishali Thakkar 2016-10-19 8:48 ` Colin Vidal 2016-10-19 8:21 ` [kernel-hardening] " Reshetova, Elena 2016-10-19 8:31 ` Greg KH 2016-10-19 8:58 ` Colin Vidal 2016-10-19 9:16 ` Greg KH 2016-10-18 14:59 ` [kernel-hardening] [RFC 2/2] arm: implementation for HARDENED_ATOMIC Colin Vidal 2016-10-18 21:29 ` [kernel-hardening] " Kees Cook 2016-10-19 8:45 ` Colin Vidal 2016-10-19 20:11 ` Kees Cook 2016-10-20 5:58 ` AKASHI Takahiro 2016-10-20 8:30 ` Colin Vidal 2016-10-25 9:18 ` AKASHI Takahiro 2016-10-25 15:02 ` Colin Vidal 2016-10-26 7:24 ` AKASHI Takahiro 2016-10-26 8:20 ` Colin Vidal 2016-10-27 11:08 ` Mark Rutland 2016-10-27 21:37 ` Kees Cook 2016-10-27 13:24 ` [kernel-hardening] " Mark Rutland 2016-10-28 5:18 ` AKASHI Takahiro 2016-10-28 8:33 ` Colin Vidal 2016-10-28 10:20 ` Mark Rutland 2016-10-28 10:59 ` David Windsor 2016-10-21 7:47 ` [kernel-hardening] Re: [RFC 0/2] arm: implementation of HARDENED_ATOMIC AKASHI Takahiro 2016-10-27 10:32 ` [kernel-hardening] " Mark Rutland 2016-10-27 12:45 ` David Windsor 2016-10-27 13:53 ` Mark Rutland 2016-10-27 14:10 ` David Windsor
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.