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

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

Hi!
<snip>

> > -ATOMIC_LONG_ADD_SUB_OP(sub, _relaxed) -ATOMIC_LONG_ADD_SUB_OP(sub, 
> > _acquire) -ATOMIC_LONG_ADD_SUB_OP(sub, _release)
> > +ATOMIC_LONG_ADD_SUB_OP(add,,)
> > +ATOMIC_LONG_ADD_SUB_OP(add,,_wrap)
> > +ATOMIC_LONG_ADD_SUB_OP(add, _relaxed,) ATOMIC_LONG_ADD_SUB_OP(add, 
> > +_acquire,) ATOMIC_LONG_ADD_SUB_OP(add, _release,)
> > +ATOMIC_LONG_ADD_SUB_OP(sub,,)
> > +//ATOMIC_LONG_ADD_SUB_OP(sub,,_wrap) todo: check if this is really 
> > +not needed
> 
> Let's get this question answered. Seems like it'd make sense to create 
> complete function coverage?

>I'd love to know the answer since there tons of variants of atomic operations.

So, the approach we have taken so far is to define only functions that are used/needed so far in the changes that we did in subsystems.
This obviously doesn't cover all possible options. I guess in order to support future changes we need a full coverage, indeed...    

<snip>

> > -ATOMIC_LONG_OP(add)
> > -ATOMIC_LONG_OP(sub)
> > -ATOMIC_LONG_OP(and)
> > -ATOMIC_LONG_OP(andnot)
> > -ATOMIC_LONG_OP(or)
> > -ATOMIC_LONG_OP(xor)
> > +ATOMIC_LONG_OP(add,)
> > +ATOMIC_LONG_OP(add,_wrap)
> > +ATOMIC_LONG_OP(sub,)
> > +ATOMIC_LONG_OP(sub,_wrap)
> > +ATOMIC_LONG_OP(and,)
> > +ATOMIC_LONG_OP(or,)
> > +ATOMIC_LONG_OP(xor,)
> > +ATOMIC_LONG_OP(andnot,)

>For instance, are we sure that we would never call atomic_long_and_wrap() to atomic_long_wrap_t variable?
Yes, same story, now we are not calling it, but things change obviously. We wanted to start somewhere, and this set is already quite big even with the current needs. 
So, does everyone agree that we should provide full coverage? 

<snip> 

> >
> > +#ifndef CONFIG_HARDENED_ATOMIC
> > +#define atomic_read_wrap(v) atomic_read(v) #define 
> > +atomic_set_wrap(v, i) atomic_set((v), (i)) #define 
> > +atomic_add_wrap(i, v) atomic_add((i), (v)) #define 
> > +atomic_add_unless_wrap(v, i, j) atomic_add_unless((v), (i), (j)) 
> > +#define atomic_sub_wrap(i, v) atomic_sub((i), (v)) #define 
> > +atomic_inc_wrap(v) atomic_inc(v) #define 
> > +atomic_inc_and_test_wrap(v) atomic_inc_and_test(v) #define 
> > +atomic_inc_return_wrap(v) atomic_inc_return(v) #define 
> > +atomic_add_return_wrap(i, v) atomic_add_return((i), (v)) #define 
> > +atomic_dec_wrap(v) atomic_dec(v) #define atomic_cmpxchg_wrap(v, o, 
> > +n) atomic_cmpxchg((v), (o), (n)) #define atomic_xchg_wrap(v, i) 
> > +atomic_xchg((v), (i)) #define atomic_long_read_wrap(v) 
> > +atomic_long_read(v) #define atomic_long_set_wrap(v, i) 
> > +atomic_long_set((v), (i)) #define atomic_long_add_wrap(i, v) 
> > +atomic_long_add((i), (v)) #define atomic_long_sub_wrap(i, v) 
> > +atomic_long_sub((i), (v)) #define atomic_long_inc_wrap(v) 
> > +atomic_long_inc(v) #define atomic_long_add_return_wrap(i, v) 
> > +atomic_long_add_return((i), (v)) #define 
> > +atomic_long_inc_return_wrap(v) atomic_long_inc_return(v) #define 
> > +atomic_long_sub_and_test_wrap(v) atomic_long_sub_and_test(v) 
> > +#define atomic_long_dec_wrap(v) atomic_long_dec(v) #define 
> > +atomic_long_xchg_wrap(v, i) atomic_long_xchg((v), (i)) #endif /* 
> > +CONFIG_HARDENED_ATOMIC */

>It seems that there are missing function definitions here if atomic_long should have all the counterparts to atomic:
>    atomic_long_add_unless_wrap()
>    atomic_long_cmpxchg_wrap()


Again, this is based on the current usage of atomic_long_wrap_t, not the full coverage. 

Best Regards,
Elena.