[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161012082634.GK19531@linaro.org>
Date: Wed, 12 Oct 2016 17:26:35 +0900
From: AKASHI Takahiro <takahiro.akashi@...aro.org>
To: kernel-hardening@...ts.openwall.com
Cc: Elena Reshetova <elena.reshetova@...el.com>,
Hans Liljestrand <ishkamiel@...il.com>,
David Windsor <dwindsor@...il.com>
Subject: Re: Re: [RFC PATCH 01/13] Add architecture
independent hardened atomic base
Hi Kees, Elena,
I've just finished prototyping HARDENED_ATOMIC support for arm64,
but
On Mon, Oct 03, 2016 at 02:10:40PM -0700, Kees Cook wrote:
> On Sun, Oct 2, 2016 at 11:41 PM, Elena Reshetova
> <elena.reshetova@...el.com> wrote:
> > This series brings the PaX/Grsecurity PAX_REFCOUNT [1]
> > feature support to the upstream kernel. All credit for the
> > feature goes to the feature authors.
> >
> > The name of the upstream feature is HARDENED_ATOMIC
> > and it is configured using CONFIG_HARDENED_ATOMIC and
> > HAVE_ARCH_HARDENED_ATOMIC.
> >
> > This series only adds x86 support; other architectures are expected
> > to add similar support gradually.
> >
> > Feature Summary
> > ---------------
> > The primary goal of KSPP is to provide protection against classes
> > of vulnerabilities. One such class of vulnerabilities, known as
> > use-after-free bugs, frequently results when reference counters
> > guarding shared kernel objects are overflowed. The existence of
> > a kernel path in which a reference counter is incremented more
> > than it is decremented can lead to wrapping. This buggy path can be
> > executed until INT_MAX/LONG_MAX is reached, at which point further
> > increments will cause the counter to wrap to 0. At this point, the
> > kernel will erroneously mark the object as not in use, resulting in
> > a multitude of undesirable cases: releasing the object to other users,
> > freeing the object while it still has legitimate users, or other
> > undefined conditions. The above scenario is known as a use-after-free
> > bug.
> >
> > HARDENED_ATOMIC provides mandatory protection against kernel
> > reference counter overflows. In Linux, reference counters
> > are implemented using the atomic_t and atomic_long_t types.
> > HARDENED_ATOMIC modifies the functions dealing with these types
> > such that when INT_MAX/LONG_MAX is reached, the atomic variables
> > remain saturated at these maximum values, rather than wrapping.
> >
> > There are several non-reference counter users of atomic_t and
> > atomic_long_t (the fact that these types are being so widely
> > misused is not addressed by this series). These users, typically
> > statistical counters, are not concerned with whether the values of
> > these types wrap, and therefore can dispense with the added performance
> > penalty incurred from protecting against overflows. New types have
> > been introduced for these users: atomic_wrap_t and atomic_long_wrap_t.
> > Functions for manipulating these types have been added as well.
> >
> > Note that the protection provided by HARDENED_ATOMIC is not "opt-in":
> > since atomic_t is so widely misused, it must be protected as-is.
> > HARDENED_ATOMIC protects all users of atomic_t and atomic_long_t
> > against overflow. New users wishing to use atomic types, but not
> > needing protection against overflows, should use the new types
> > introduced by this series: atomic_wrap_t and atomic_long_wrap_t.
> >
> > Bugs Prevented
> > --------------
> > HARDENED_ATOMIC would directly mitigate these Linux kernel bugs:
> >
> > CVE-2016-3135 - Netfilter xt_alloc_table_info integer overflow
> > CVE-2016-0728 - Keyring refcount overflow
> > CVE-2014-2851 - Group_info refcount overflow
> > CVE-2010-2959 - CAN integer overflow vulnerability,
> > related post: https://jon.oberheide.org/blog/2010/09/10/linux-kernel-can-slub-overflow/
> >
> > And a relatively fresh exploit example:
> > https://www.exploit-db.com/exploits/39773/
> >
> > [1] https://forums.grsecurity.net/viewtopic.php?f=7&t=4173
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@...el.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@...il.com>
> > Signed-off-by: David Windsor <dwindsor@...il.com>
> > ---
> > include/asm-generic/atomic-long.h | 166 +++++++++++++++++++++++++++-----------
> > include/asm-generic/atomic.h | 9 +++
> > include/asm-generic/atomic64.h | 13 +++
> > include/asm-generic/bug.h | 4 +
> > include/asm-generic/local.h | 15 ++++
> > include/linux/atomic.h | 14 ++++
> > include/linux/types.h | 17 ++++
> > kernel/panic.c | 12 +++
> > security/Kconfig | 15 ++++
> > 9 files changed, 219 insertions(+), 46 deletions(-)
> >
> > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
> > index 288cc9e..e74ca86 100644
> > --- a/include/asm-generic/atomic-long.h
> > +++ b/include/asm-generic/atomic-long.h
> > @@ -22,6 +22,12 @@
> >
> > typedef atomic64_t atomic_long_t;
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +typedef atomic64_wrap_t atomic_long_wrap_t;
> > +#else
> > +typedef atomic64_t atomic_long_wrap_t;
> > +#endif
> > +
> > #define ATOMIC_LONG_INIT(i) ATOMIC64_INIT(i)
> > #define ATOMIC_LONG_PFX(x) atomic64 ## x
> >
> > @@ -29,51 +35,61 @@ typedef atomic64_t atomic_long_t;
> >
> > typedef atomic_t atomic_long_t;
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +typedef atomic_wrap_t atomic_long_wrap_t;
> > +#else
> > +typedef atomic_t atomic_long_wrap_t;
> > +#endif
> > +
> > #define ATOMIC_LONG_INIT(i) ATOMIC_INIT(i)
> > #define ATOMIC_LONG_PFX(x) atomic ## x
> >
> > #endif
> >
> > -#define ATOMIC_LONG_READ_OP(mo) \
> > -static inline long atomic_long_read##mo(const atomic_long_t *l) \
> > +#define ATOMIC_LONG_READ_OP(mo, suffix) \
> > +static inline long atomic_long_read##mo##suffix(const atomic_long##suffix##_t *l)\
> > { \
> > - ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l; \
> > + ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\
> > \
> > - return (long)ATOMIC_LONG_PFX(_read##mo)(v); \
> > + return (long)ATOMIC_LONG_PFX(_read##mo##suffix)(v); \
> > }
> > -ATOMIC_LONG_READ_OP()
> > -ATOMIC_LONG_READ_OP(_acquire)
> > +ATOMIC_LONG_READ_OP(,)
> > +ATOMIC_LONG_READ_OP(,_wrap)
> > +ATOMIC_LONG_READ_OP(_acquire,)
> >
> > #undef ATOMIC_LONG_READ_OP
> >
> > -#define ATOMIC_LONG_SET_OP(mo) \
> > -static inline void atomic_long_set##mo(atomic_long_t *l, long i) \
> > +#define ATOMIC_LONG_SET_OP(mo, suffix) \
> > +static inline void atomic_long_set##mo##suffix(atomic_long##suffix##_t *l, long i)\
> > { \
> > - ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l; \
> > + ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\
> > \
> > - ATOMIC_LONG_PFX(_set##mo)(v, i); \
> > + ATOMIC_LONG_PFX(_set##mo##suffix)(v, i); \
> > }
> > -ATOMIC_LONG_SET_OP()
> > -ATOMIC_LONG_SET_OP(_release)
> > +ATOMIC_LONG_SET_OP(,)
> > +ATOMIC_LONG_SET_OP(,_wrap)
> > +ATOMIC_LONG_SET_OP(_release,)
> >
> > #undef ATOMIC_LONG_SET_OP
> >
> > -#define ATOMIC_LONG_ADD_SUB_OP(op, mo) \
> > +#define ATOMIC_LONG_ADD_SUB_OP(op, mo, suffix) \
> > static inline long \
> > -atomic_long_##op##_return##mo(long i, atomic_long_t *l) \
> > +atomic_long_##op##_return##mo##suffix(long i, atomic_long##suffix##_t *l)\
> > { \
> > - ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l; \
> > + ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\
> > \
> > - return (long)ATOMIC_LONG_PFX(_##op##_return##mo)(i, v); \
> > + return (long)ATOMIC_LONG_PFX(_##op##_return##mo##suffix)(i, v);\
> > }
> > -ATOMIC_LONG_ADD_SUB_OP(add,)
> > -ATOMIC_LONG_ADD_SUB_OP(add, _relaxed)
> > -ATOMIC_LONG_ADD_SUB_OP(add, _acquire)
> > -ATOMIC_LONG_ADD_SUB_OP(add, _release)
> > -ATOMIC_LONG_ADD_SUB_OP(sub,)
> > -ATOMIC_LONG_ADD_SUB_OP(sub, _relaxed)
> > -ATOMIC_LONG_ADD_SUB_OP(sub, _acquire)
> > -ATOMIC_LONG_ADD_SUB_OP(sub, _release)
> > +ATOMIC_LONG_ADD_SUB_OP(add,,)
> > +ATOMIC_LONG_ADD_SUB_OP(add,,_wrap)
> > +ATOMIC_LONG_ADD_SUB_OP(add, _relaxed,)
> > +ATOMIC_LONG_ADD_SUB_OP(add, _acquire,)
> > +ATOMIC_LONG_ADD_SUB_OP(add, _release,)
> > +ATOMIC_LONG_ADD_SUB_OP(sub,,)
> > +//ATOMIC_LONG_ADD_SUB_OP(sub,,_wrap) todo: check if this is really not needed
>
> Let's get this question answered. Seems like it'd make sense to create
> complete function coverage?
I'd love to know the answer since there tons of variants of atomic operations.
> > +ATOMIC_LONG_ADD_SUB_OP(sub, _relaxed,)
> > +ATOMIC_LONG_ADD_SUB_OP(sub, _acquire,)
> > +ATOMIC_LONG_ADD_SUB_OP(sub, _release,)
> >
> > #undef ATOMIC_LONG_ADD_SUB_OP
> >
> > @@ -98,6 +114,11 @@ ATOMIC_LONG_ADD_SUB_OP(sub, _release)
> > #define atomic_long_xchg(v, new) \
> > (ATOMIC_LONG_PFX(_xchg)((ATOMIC_LONG_PFX(_t) *)(v), (new)))
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +#define atomic_long_xchg_wrap(v, new) \
> > + (ATOMIC_LONG_PFX(_xchg_wrap)((ATOMIC_LONG_PFX(_wrap_t) *)(v), (new)))
> > +#endif
> > +
> > static __always_inline void atomic_long_inc(atomic_long_t *l)
> > {
> > ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l;
> > @@ -105,6 +126,15 @@ static __always_inline void atomic_long_inc(atomic_long_t *l)
> > ATOMIC_LONG_PFX(_inc)(v);
> > }
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +static __always_inline void atomic_long_inc_wrap(atomic_long_wrap_t *l)
> > +{
> > + ATOMIC_LONG_PFX(_wrap_t) *v = (ATOMIC_LONG_PFX(_wrap_t) *)l;
> > +
> > + ATOMIC_LONG_PFX(_inc_wrap)(v);
> > +}
> > +#endif
> > +
> > static __always_inline void atomic_long_dec(atomic_long_t *l)
> > {
> > ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l;
> > @@ -112,6 +142,15 @@ static __always_inline void atomic_long_dec(atomic_long_t *l)
> > ATOMIC_LONG_PFX(_dec)(v);
> > }
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +static __always_inline void atomic_long_dec_wrap(atomic_long_wrap_t *l)
> > +{
> > + ATOMIC_LONG_PFX(_wrap_t) *v = (ATOMIC_LONG_PFX(_wrap_t) *)l;
> > +
> > + ATOMIC_LONG_PFX(_dec_wrap)(v);
> > +}
> > +#endif
> > +
> > #define ATOMIC_LONG_FETCH_OP(op, mo) \
> > static inline long \
> > atomic_long_fetch_##op##mo(long i, atomic_long_t *l) \
> > @@ -168,21 +207,23 @@ ATOMIC_LONG_FETCH_INC_DEC_OP(dec, _release)
> >
> > #undef ATOMIC_LONG_FETCH_INC_DEC_OP
> >
> > -#define ATOMIC_LONG_OP(op) \
> > +#define ATOMIC_LONG_OP(op, suffix) \
> > static __always_inline void \
> > -atomic_long_##op(long i, atomic_long_t *l) \
> > +atomic_long_##op##suffix(long i, atomic_long##suffix##_t *l) \
> > { \
> > - ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l; \
> > + ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\
> > \
> > - ATOMIC_LONG_PFX(_##op)(i, v); \
> > + ATOMIC_LONG_PFX(_##op##suffix)(i, v); \
> > }
> >
> > -ATOMIC_LONG_OP(add)
> > -ATOMIC_LONG_OP(sub)
> > -ATOMIC_LONG_OP(and)
> > -ATOMIC_LONG_OP(andnot)
> > -ATOMIC_LONG_OP(or)
> > -ATOMIC_LONG_OP(xor)
> > +ATOMIC_LONG_OP(add,)
> > +ATOMIC_LONG_OP(add,_wrap)
> > +ATOMIC_LONG_OP(sub,)
> > +ATOMIC_LONG_OP(sub,_wrap)
> > +ATOMIC_LONG_OP(and,)
> > +ATOMIC_LONG_OP(or,)
> > +ATOMIC_LONG_OP(xor,)
> > +ATOMIC_LONG_OP(andnot,)
For instance, are we sure that we would never call atomic_long_and_wrap()
to atomic_long_wrap_t variable?
> > #undef ATOMIC_LONG_OP
> >
> > @@ -193,6 +234,13 @@ static inline int atomic_long_sub_and_test(long i, atomic_long_t *l)
> > return ATOMIC_LONG_PFX(_sub_and_test)(i, v);
> > }
> >
> > +static inline int atomic_long_sub_and_test_wrap(long i, atomic_long_wrap_t *l)
> > +{
> > + ATOMIC_LONG_PFX(_wrap_t) *v = (ATOMIC_LONG_PFX(_wrap_t) *)l;
> > +
> > + return ATOMIC_LONG_PFX(_sub_and_test_wrap)(i, v);
> > +}
> > +
> > static inline int atomic_long_dec_and_test(atomic_long_t *l)
> > {
> > ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l;
> > @@ -214,22 +262,23 @@ static inline int atomic_long_add_negative(long i, atomic_long_t *l)
> > return ATOMIC_LONG_PFX(_add_negative)(i, v);
> > }
> >
> > -#define ATOMIC_LONG_INC_DEC_OP(op, mo) \
> > +#define ATOMIC_LONG_INC_DEC_OP(op, mo, suffix) \
> > static inline long \
> > -atomic_long_##op##_return##mo(atomic_long_t *l) \
> > +atomic_long_##op##_return##mo##suffix(atomic_long##suffix##_t *l) \
> > { \
> > - ATOMIC_LONG_PFX(_t) *v = (ATOMIC_LONG_PFX(_t) *)l; \
> > + ATOMIC_LONG_PFX(suffix##_t) *v = (ATOMIC_LONG_PFX(suffix##_t) *)l;\
> > \
> > - return (long)ATOMIC_LONG_PFX(_##op##_return##mo)(v); \
> > + return (long)ATOMIC_LONG_PFX(_##op##_return##mo##suffix)(v); \
> > }
> > -ATOMIC_LONG_INC_DEC_OP(inc,)
> > -ATOMIC_LONG_INC_DEC_OP(inc, _relaxed)
> > -ATOMIC_LONG_INC_DEC_OP(inc, _acquire)
> > -ATOMIC_LONG_INC_DEC_OP(inc, _release)
> > -ATOMIC_LONG_INC_DEC_OP(dec,)
> > -ATOMIC_LONG_INC_DEC_OP(dec, _relaxed)
> > -ATOMIC_LONG_INC_DEC_OP(dec, _acquire)
> > -ATOMIC_LONG_INC_DEC_OP(dec, _release)
> > +ATOMIC_LONG_INC_DEC_OP(inc,,)
> > +ATOMIC_LONG_INC_DEC_OP(inc,,_wrap)
> > +ATOMIC_LONG_INC_DEC_OP(inc, _relaxed,)
> > +ATOMIC_LONG_INC_DEC_OP(inc, _acquire,)
> > +ATOMIC_LONG_INC_DEC_OP(inc, _release,)
> > +ATOMIC_LONG_INC_DEC_OP(dec,,)
> > +ATOMIC_LONG_INC_DEC_OP(dec, _relaxed,)
> > +ATOMIC_LONG_INC_DEC_OP(dec, _acquire,)
> > +ATOMIC_LONG_INC_DEC_OP(dec, _release,)
> >
> > #undef ATOMIC_LONG_INC_DEC_OP
> >
> > @@ -243,4 +292,29 @@ 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(v) atomic_long_sub_and_test(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 */
It seems that there are missing function definitions here if atomic_long
should have all the counterparts to atomic:
atomic_long_add_unless_wrap()
atomic_long_cmpxchg_wrap()
Thanks,
-Takahiro AKASHI
> > +
> > #endif /* _ASM_GENERIC_ATOMIC_LONG_H */
> > diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
> > index 9ed8b98..159e749 100644
> > --- a/include/asm-generic/atomic.h
> > +++ b/include/asm-generic/atomic.h
> > @@ -232,4 +232,13 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u)
> > return c;
> > }
> >
> > +static inline int __atomic_add_unless_wrap(atomic_t *v, int a, int u)
> > +{
> > + int c, old;
> > + c = atomic_read_wrap(v);
> > + while (c != u && (old = atomic_cmpxchg_wrap(v, c, c + a)) != c)
> > + c = old;
> > + return c;
> > +}
> > +
> > #endif /* __ASM_GENERIC_ATOMIC_H */
> > diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h
> > index dad68bf..4987419 100644
> > --- a/include/asm-generic/atomic64.h
> > +++ b/include/asm-generic/atomic64.h
> > @@ -16,6 +16,8 @@ typedef struct {
> > long long counter;
> > } atomic64_t;
> >
> > +typedef atomic64_t atomic64_wrap_t;
> > +
> > #define ATOMIC64_INIT(i) { (i) }
> >
> > extern long long atomic64_read(const atomic64_t *v);
> > @@ -62,4 +64,15 @@ extern 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_read_wrap(v) atomic64_read(v)
> > +#define atomic64_set_wrap(v, i) atomic64_set((v), (i))
> > +#define atomic64_add_wrap(a, v) atomic64_add((a), (v))
> > +#define atomic64_add_return_wrap(a, v) atomic64_add_return((a), (v))
> > +#define atomic64_sub_wrap(a, v) atomic64_sub((a), (v))
> > +#define atomic64_inc_wrap(v) atomic64_inc(v)
> > +#define atomic64_inc_return_wrap(v) atomic64_inc_return(v)
> > +#define atomic64_dec_wrap(v) atomic64_dec(v)
> > +#define atomic64_cmpxchg_wrap(v, o, n) atomic64_cmpxchg((v), (o), (n))
> > +#define atomic64_xchg_wrap(v, n) atomic64_xchg((v), (n))
> > +
> > #endif /* _ASM_GENERIC_ATOMIC64_H */
> > diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> > index 6f96247..9428d60 100644
> > --- a/include/asm-generic/bug.h
> > +++ b/include/asm-generic/bug.h
> > @@ -215,6 +215,10 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
> > # define WARN_ON_SMP(x) ({0;})
> > #endif
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +void hardened_atomic_refcount_overflow(struct pt_regs *regs);
> > +#endif
>
> This should probably drop the "refcount" part of its name: refcounting
> is higher level, and this is just simply reporting the overflow
> condition. And that would make the name shorter. Additionally, I think
> an #else case should be added for a no-op static inline to make
> calling this in the per-arch traps avoid using an #ifdef.
>
> > +
> > #endif /* __ASSEMBLY__ */
> >
> > #endif
> > diff --git a/include/asm-generic/local.h b/include/asm-generic/local.h
> > index 9ceb03b..a98ad1d 100644
> > --- a/include/asm-generic/local.h
> > +++ b/include/asm-generic/local.h
> > @@ -23,24 +23,39 @@ typedef struct
> > atomic_long_t a;
> > } local_t;
> >
> > +typedef struct {
> > + atomic_long_wrap_t a;
> > +} local_wrap_t;
> > +
> > #define LOCAL_INIT(i) { ATOMIC_LONG_INIT(i) }
> >
> > #define local_read(l) atomic_long_read(&(l)->a)
> > +#define local_read_wrap(l) atomic_long_read_wrap(&(l)->a)
> > #define local_set(l,i) atomic_long_set((&(l)->a),(i))
> > +#define local_set_wrap(l,i) atomic_long_set_wrap((&(l)->a),(i))
> > #define local_inc(l) atomic_long_inc(&(l)->a)
> > +#define local_inc_wrap(l) atomic_long_inc_wrap(&(l)->a)
> > #define local_dec(l) atomic_long_dec(&(l)->a)
> > +#define local_dec_wrap(l) atomic_long_dec_wrap(&(l)->a)
> > #define local_add(i,l) atomic_long_add((i),(&(l)->a))
> > +#define local_add_wrap(i,l) atomic_long_add_wrap((i),(&(l)->a))
> > #define local_sub(i,l) atomic_long_sub((i),(&(l)->a))
> > +#define local_sub_wrap(i,l) atomic_long_sub_wrap((i),(&(l)->a))
> >
> > #define local_sub_and_test(i, l) atomic_long_sub_and_test((i), (&(l)->a))
> > +#define local_sub_and_test_wrap(i, l) atomic_long_sub_and_test_wrap((i), (&(l)->a))
> > #define local_dec_and_test(l) atomic_long_dec_and_test(&(l)->a)
> > #define local_inc_and_test(l) atomic_long_inc_and_test(&(l)->a)
> > #define local_add_negative(i, l) atomic_long_add_negative((i), (&(l)->a))
> > #define local_add_return(i, l) atomic_long_add_return((i), (&(l)->a))
> > +#define local_add_return_wrap(i, l) atomic_long_add_return_wrap((i), (&(l)->a))
> > #define local_sub_return(i, l) atomic_long_sub_return((i), (&(l)->a))
> > #define local_inc_return(l) atomic_long_inc_return(&(l)->a)
> > +/* verify that below function is needed */
> > +#define local_dec_return(l) atomic_long_dec_return(&(l)->a)
> >
> > #define local_cmpxchg(l, o, n) atomic_long_cmpxchg((&(l)->a), (o), (n))
> > +#define local_cmpxchg_wrap(l, o, n) atomic_long_cmpxchg_wrap((&(l)->a), (o), (n))
> > #define local_xchg(l, n) atomic_long_xchg((&(l)->a), (n))
> > #define local_add_unless(l, _a, u) atomic_long_add_unless((&(l)->a), (_a), (u))
> > #define local_inc_not_zero(l) atomic_long_inc_not_zero(&(l)->a)
> > diff --git a/include/linux/atomic.h b/include/linux/atomic.h
> > index e71835b..b5817c8 100644
> > --- a/include/linux/atomic.h
> > +++ b/include/linux/atomic.h
> > @@ -507,6 +507,20 @@ static inline int atomic_add_unless(atomic_t *v, int a, int u)
> > }
> >
> > /**
> > + * atomic_add_unless_wrap - add unless the number is already a given value
> > + * @v: pointer of type atomic_wrap_t
> > + * @a: the amount to add to v...
> > + * @u: ...unless v is equal to u.
> > + *
> > + * Atomically adds @a to @v, so long as @v was not already @u.
> > + * Returns non-zero if @v was not @u, and zero otherwise.
> > + */
> > +static inline int atomic_add_unless_wrap(atomic_wrap_t *v, int a, int u)
> > +{
> > + return __atomic_add_unless_wrap(v, a, u) != u;
> > +}
> > +
> > +/**
> > * atomic_inc_not_zero - increment unless the number is zero
> > * @v: pointer of type atomic_t
> > *
> > diff --git a/include/linux/types.h b/include/linux/types.h
> > index baf7183..b47a7f8 100644
> > --- a/include/linux/types.h
> > +++ b/include/linux/types.h
> > @@ -175,10 +175,27 @@ typedef struct {
> > int counter;
> > } atomic_t;
> >
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +typedef struct {
> > + int counter;
> > +} atomic_wrap_t;
> > +#else
> > +typedef atomic_t atomic_wrap_t;
> > +#endif
> > +
> > #ifdef CONFIG_64BIT
> > typedef struct {
> > long counter;
> > } atomic64_t;
> > +
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +typedef struct {
> > + long counter;
> > +} atomic64_wrap_t;
> > +#else
> > +typedef atomic64_t atomic64_wrap_t;
> > +#endif
> > +
> > #endif
> >
> > struct list_head {
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index e6480e2..66b0154 100644
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -616,3 +616,15 @@ static int __init oops_setup(char *s)
> > return 0;
> > }
> > early_param("oops", oops_setup);
> > +
> > +#ifdef CONFIG_HARDENED_ATOMIC
> > +void hardened_atomic_refcount_overflow(struct pt_regs *regs)
> > +{
> > + printk(KERN_EMERG "HARDENED_ATOMIC: refcount overflow detected in: %s:%d, uid/euid: %u/%u\n",
>
> This can be pr_emerg(). I'd drop the "refcount" mention here and below.
>
> > + current->comm, task_pid_nr(current),
> > + from_kuid_munged(&init_user_ns, current_uid()),
> > + from_kuid_munged(&init_user_ns, current_euid()));
> > + print_symbol(KERN_EMERG "HARDENED_ATOMIC: refcount overflow occurred at: %s\n", instruction_pointer(regs));
> > + BUG();
>
> Does print_symbol() produce more information than BUG() here?
>
> > +}
> > +#endif
> > diff --git a/security/Kconfig b/security/Kconfig
> > index 118f454..dc39f7e 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -158,6 +158,21 @@ config HARDENED_USERCOPY_PAGESPAN
> > been removed. This config is intended to be used only while
> > trying to find such users.
> >
> > +config HAVE_ARCH_HARDENED_ATOMIC
> > + bool
> > + help
> > + The architecture supports CONFIG_HARDENED_ATOMIC by
> > + providing additional checks on counter overflows for atomic_t
> > +
> > +config HARDENED_ATOMIC
> > + bool "Prevent reference counter overflow in atomic_t"
> > + depends on HAVE_ARCH_HARDENED_ATOMIC
>
> Oh, this should select BUG too.
>
> > + help
> > + This option prevents reference counters in atomic_t from
> > + overflow. This allows to avoid the
> > + situation when counter overflow leads to an exploitable
> > + use-after-free situation.
>
> I think the Kconfig help text could be clarified up a bit (and needs
> some minor formatting adjustments). Perhaps something like:
>
> config HAVE_ARCH_HARDENED_ATOMIC
> ...
> The architecture supports CONFIG_HARDENED_ATOMIC by
> providing trapping on atomic_t wraps, with a call to
> hardened_atomic_overflow().
>
> config HARDENED_ATOMIC
> ...
> This option catches counter wrapping in atomic_t, which
> can turn refcounting over/underflow bugs into resource
> consumption bugs instead of exploitable user-after-free flaws.
>
> > +
> > source security/selinux/Kconfig
> > source security/smack/Kconfig
> > source security/tomoyo/Kconfig
> > --
> > 2.7.4
> >
>
> -Kees
>
> --
> Kees Cook
> Nexus Security
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
