[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jLoQY4yg0zfDOiW6MsFQ5xQ9i8GG5sdU2=G69xqEbxL0g@mail.gmail.com>
Date: Wed, 26 Oct 2016 12:49:49 -0700
From: Kees Cook <keescook@...omium.org>
To: "Reshetova, Elena" <elena.reshetova@...el.com>
Cc: Colin Vidal <colin@...dal.org>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, David Windsor <dave@...gbits.org>
Subject: Re: [RFC v2 PATCH 00/13] HARDENED_ATOMIC
On Wed, Oct 26, 2016 at 2:46 AM, Reshetova, Elena
<elena.reshetova@...el.com> wrote:
> So, again for clarity, let's break this case into two cases also:
> (3) CONFIG_GENERIC_ATOMIC64 is set, architecture does not provide any
> atomic64 implementations, CONFIG_ARCH_HARDENED_ATOMIC is set
> (4) CONFIG_GENERIC_ATOMIC64 is set, architecture does not provide any
> atomic64 implementations, CONFIG_ARCH_HARDENED_ATOMIC is unset
>
> The current implementation what we have in the hardened_atomic_on_next
> branch actually treats both of these cases in the same way (this is what
> Hans was explaining before): it does define atomic64*_wrap functions in
> both cases, but they point to the same normal functions. So, no protection
> provided in any case. Based on the current thread it seems that we might
> want to have protection in the case (3) after all, which is not really
> straightforward (maybe that's why grsecurity didn't have support for it).
3 is an interesting combination, and a seemingly strange hole in
PAX_REFCOUNT. The implementation of CONFIG_GENERIC_ATOMIC64 uses
spinlocks, so doing an overflow test would be trivial to add. I was
thinking that this wasn't done under PAX_REFCOUNT because the CONFIG
combination didn't exist, but that seems false:
config ARM
...
select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
config PAX_REFCOUNT
...
depends on GRKERNSEC && ((ARM && (CPU_V6 || CPU_V6K ||
CPU_V7)) || MIPS || PPC || SPARC64 || X86)
The ARM && CPU_V6 case is explicitly covered by PAX_REFCOUNT, but
AFACIT, doesn't protect atomic64.
> Also, what's why we first wanted to know how common this case (3)
> would be in practice? Should we just merge first the common cases and
> for example as Colin was proposing make it impossible for now to select
> CONFIG_HARDENED_ATOMIC for the case (3)? Actually I believe it is not
> even really possible to select it now in this case, or am I wrong? IMO
> we don't have anything providing CONFIG_ARCH_HARDENED_ATOMIC in this case.
I think a fine first step would be to have CONFIG_HARDENED_ATOMIC
depend on !GENERIC_ATOMIC64 for now. When HARDENED_ATOMIC is expanded
to include other architectures that use GENERIC_ATOMIC64, it can be
added then. (Which maybe Colin would be interested in doing for ARM
CPU_V6.)
-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.
