kernel-hardening - Re: [RFC v3 PATCH 01/13] Add architecture independent hardened atomic base

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161101113542.rofdy3cbgt6c3cvp@thigreal>
Date: Tue, 1 Nov 2016 13:35:42 +0200
From: Hans Liljestrand <ishkamiel@...il.com>
To: "Reshetova, Elena" <elena.reshetova@...el.com>
Cc: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
	"AKASHI, Takahiro" <takahiro.akashi@...aro.org>,
	"keescook@...omium.org" <keescook@...omium.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 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.

Thanks,
-hans

> 
> Best Regards,
> Elena.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.