[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2236FBA76BA1254E88B949DDB74E612B41BFC2F6@IRSMSX102.ger.corp.intel.com>
Date: Wed, 2 Nov 2016 04:55:35 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: Kees Cook <keescook@...omium.org>, Hans Liljestrand <ishkamiel@...il.com>
CC: "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).
I like it this way. And then the atomic_wrap_t can be defined as a struct of its own even in case of hardening atomic is off.
We will include this for rfc v4!
Best Regards,
Elena.
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.
