[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 1 Nov 2016 06:57:08 -0700
From: Kees Cook <keescook@...omium.org>
To: Hans Liljestrand <ishkamiel@...il.com>
Cc: "Reshetova, Elena" <elena.reshetova@...el.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
"AKASHI, Takahiro" <takahiro.akashi@...aro.org>, "arnd@...db.de" <arnd@...db.de>,
"tglx@...utronix.de" <tglx@...utronix.de>, "mingo@...hat.com" <mingo@...hat.com>,
"Anvin, H Peter" <h.peter.anvin@...el.com>, David Windsor <dwindsor@...il.com>
Subject: Re: [RFC v3 PATCH 01/13] Add architecture
independent hardened atomic base
On Tue, Nov 1, 2016 at 4:35 AM, Hans Liljestrand <ishkamiel@...il.com> wrote:
> On Tue, Nov 01, 2016 at 10:59:03AM +0000, Reshetova, Elena wrote:
>> >Hi Elena,
>>
>> Hi!
>>
>> <snip>
>>
>> > 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
>> >
>>
>> >I still think it would be a good idea to always distinct atomic*_wrap_t and atomic_t. Otherwise, it is possible to mix those two types without getting any error, if CONFIG_HARDENED_ATOMIC is disabled (no big deal in that case, since there is no protection anyways, but it is quite unclean...). What do you think?
>>
>> Yes, now after we hopefully have all the coverage done and other issues resolved, it might be time to address this.
>> But I am not 100% sure what is the proper way. Defining the same struct type as for hardening case?
>
> Hi Colin, Elena,
>
> Wouldn't that also necessitate that we provide implementations for the wrap
> functions? As it is now, we can just define the _wrap functions as macros
> pointing the plain functions, but if the types are distinct that wouldn't work
> (which, if I understand correctly, is the whole point).
>
> So then we'd need to add default implementations for the _wrap functions. And
> wouldn't those essentially have to use the arch implemented plain function to
> ensure it actually works in all cases?
>
> So I'm not disagreeing with you, I'm just not clear on the proper/clean way to
> approach this either.
I think the not-hardened _wrap functions would just look like this:
static inline atomic_inc_wrap(atomic_wrap_t *atomic) {
atomic_inc((atomic_wrap *)atomic);
}
Then type checking is done with or without CONFIG_HARDENED_ATOMIC, and
there is no change in binary code size (the compiler will just replace
the call).
-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.
