Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 3 Oct 2016 14:38:24 -0700
From: Kees Cook <keescook@...omium.org>
To: David Windsor <dwindsor@...il.com>
Cc: Elena Reshetova <elena.reshetova@...el.com>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, 
	Hans Liljestrand <ishkamiel@...il.com>
Subject: Re: [RFC PATCH 01/13] Add architecture independent hardened atomic base

On Mon, Oct 3, 2016 at 2:26 PM, David Windsor <dwindsor@...il.com> wrote:
> On Mon, Oct 3, 2016 at 5:10 PM, Kees Cook <keescook@...omium.org> wrote:
>
>> > +}
>> > +#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.
>>
>
> Sorry to be pedantic, but this feature doesn't actually protect
> against underflowing atomic_t, and you actually meant "use-after-free"
> flaws.

Oh right, thank you! So, better would be:

   This option catches counter wrapping in atomic_t, which
   can turn refcounting overflow bugs into resource
   consumption bugs instead of exploitable user-after-free flaws.

> Do we want to mention something about the negligible performance
> impact, as is done in the original PAX_REFCOUNT feature:
>   Since this has a negligible performance impact, you should enable
> this feature.
>
> Not sure if we eventually envision this feature being on-by-default;
> if not, it may be useful to include this text.

Yeah, actually, the performance impact should be mentioned. I'd be
good to actually try to measure this, just to have looked at a
specific workload.

This could also be the basis for Dave Hanson's suggestion to use
cmpchxg: would the expense of that instead of locked addl get noticed,
etc?

-Kees

>
>> > +
>> >  source security/selinux/Kconfig
>> >  source security/smack/Kconfig
>> >  source security/tomoyo/Kconfig
>> > --
>> > 2.7.4
>> >
>>
>> -Kees
>>
>> --
>> Kees Cook
>> Nexus Security



-- 
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.