[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 19 Oct 2016 13:11:46 -0700
From: Kees Cook <keescook@...omium.org>
To: Colin Vidal <colin@...dal.org>
Cc: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
"Reshetova, Elena" <elena.reshetova@...el.com>, AKASHI Takahiro <takahiro.akashi@...aro.org>,
David Windsor <dave@...gbits.org>, Hans Liljestrand <ishkamiel@...il.com>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [RFC 2/2] arm: implementation for HARDENED_ATOMIC
On Wed, Oct 19, 2016 at 1:45 AM, Colin Vidal <colin@...dal.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@...dal.org>
>> > ---
>> > arch/arm/Kconfig | 1 +
>> > arch/arm/include/asm/atomic.h | 434 +++++++++++++++++++++++++++++-------------
>> > arch/arm/mm/fault.c | 15 ++
>> > 3 files changed, 320 insertions(+), 130 deletions(-)
>> >
>> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> > index b5d529f..fcf4a64 100644
>> > --- a/arch/arm/Kconfig
>> > +++ b/arch/arm/Kconfig
>> > @@ -36,6 +36,7 @@ config ARM
>> > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
>> > select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
>> > select HAVE_ARCH_HARDENED_USERCOPY
>> > + select HAVE_ARCH_HARDENED_ATOMIC
>> > select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
>> > select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
>> > select HAVE_ARCH_MMAP_RND_BITS if MMU
>> > diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
>> > index 66d0e21..fdaee17 100644
>> > --- a/arch/arm/include/asm/atomic.h
>> > +++ b/arch/arm/include/asm/atomic.h
>> > @@ -17,18 +17,52 @@
>> > #include <linux/irqflags.h>
>> > #include <asm/barrier.h>
>> > #include <asm/cmpxchg.h>
>> > +#include <linux/bug.h>
>> >
>> > #define ATOMIC_INIT(i) { (i) }
>> >
>> > #ifdef __KERNEL__
>> >
>> > +#ifdef CONFIG_HARDENED_ATOMIC
>> > +#define HARDENED_ATOMIC_INSN "bkpt 0xf103"
>>
>> 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
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.
