RE: [kernel-hardening] [RFC v2 PATCH 00/13] HARDENED_ATOMIC

["Reshetova, Elena" <elena.reshetova@intel.com>]

>On Wed, 2016-10-26 at 12:52 -0700, Kees Cook wrote:
> On Wed, Oct 26, 2016 at 12:47 PM, Colin Vidal <colin@cvidal.org> wrote:
> > 
> > > 
> > > BTW, I just looked to the generic implementation of atomic64. It 
> > > seems quite understandable: methods use spinlock to access/modify 
> > > to the value of an atomic64 variable. It seems possible to check 
> > > the value before the increment/decrements and if the resulting 
> > > value is 0, but the value before the operation was different of -1 
> > > or 1, is that an overflow just happened (well, it is not exactly 
> > > right, but this is the global idea). Hence, we revert the change, 
> > > release the lock, and kill the process.
> > > 
> > > If this idea is correct, it would avoid specific implementation of 
> > > protected version of atomic64 for architecture with 
> > > GENERIC_ATOMIC64. And case (3) would be easily protected. What do 
> > > you think?
> > 
> > What I am saying here is quite confusing. Here is a cleaner
> > explanation:
> > 
> >  * the generic atomic64 method enter and takes the lock
> >  * before making the operation, check v->counter > INT_MAX - value 
> > (ifadd) or check v->counter < INT_MIN - value (if sub)
> >  * if the previous check is true, release the lock and kill the 
> > process
> >  * otherwise, let the operation process.
> > 
> > Obviously, if this approach is not wrong, there will be a 
> > significant overhead, but it happens only on CONFIG_GENERIC_ATOMIC64 
> > && CONFIG_HARDENED_ATOMIC.
> 
> I think this would be fine -- though I think it should be a distinct 
> patch. Anything we can do to separate changes into logical chunks 
> makes reviewing easier.
> 
> i.e. patch ordering could look like this:
> 
> - original series with HARDENED_ATOMIC depending on !GENERIC_ATOMIC64
> - implementation of protection on GENERIC_ATOMIC64, removing above 
> depends limitation
> - ARM hardened atomic implementation

>Great!

>Elena, I will wait that you applies HARDENED_ATOMIC depending on !GENERIC_ATOMIC64, and I submit a new RFC with the implementation of protection on GENERIC_ATOMIC64 and a v2 of ARM port. Sounds good for everybody? 

Change pushed. Now it should be !GENERIC_ATOMIC64. Hopefully this for now concludes our state on atomic64* variables. 

Now we are left with local_wrap_t problem still... But it doesn’t concern arm I think at all. 

Best Regards,
Elena.