Hi Jules,

On Thu, Jan 26, 2023 at 10:57:20AM +0100, Jules Maselbas wrote:
> Hi Mark,
> 
> On Fri, Jan 20, 2023 at 03:18:48PM +0000, Mark Rutland wrote:
> > On Fri, Jan 20, 2023 at 03:09:42PM +0100, Yann Sionneau wrote:
> > > +#define ATOMIC64_RETURN_OP(op, c_op)                                     
> > > \
> > > +static inline long arch_atomic64_##op##_return(long i, atomic64_t *v)    
> > > \
> > > +{                                                                        
> > > \
> > > + long new, old, ret;                                             \
> > > +                                                                 \
> > > + do {                                                            \
> > > +         old = v->counter;                                       \
> > 
> > This should be arch_atomic64_read(v), in order to avoid the potential for 
> > the
> > compiler to replay the access and introduce ABA races and other such 
> > problems.
> Thanks for the suggestion, this will be into v3.
> 
> > For details, see:
> > 
> >   https://lore.kernel.org/lkml/Y70SWXHDmOc3RhMd@osiris/
> >   https://lore.kernel.org/lkml/Y71LoCIl+IFdy9D8@FVFF77S0Q05N/
> > 
> > I see that the generic 32-bit atomic code suffers from that issue, and we
> > should fix it.
> I took a look at the generic 32-bit atomic, but I am unsure if this
> needs to be done for both the SMP and non-SMP implementations. But I
> can send a first patch and we can discuss from there.

Sounds good to me; thanks!

[...]

> > > +static inline int arch_atomic_add_return(int i, atomic_t *v)
> > > +{
> > > + int new, old, ret;
> > > +
> > > + do {
> > > +         old = v->counter;
> > 
> > Likewise, arch_atomic64_read(v) here.
> ack, this will bt arch_atomic_read(v) here since this is not atomic64_t
> here.

Ah, yes, my bad!

Thanks,
Mark.

--
Linux-audit mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/linux-audit

Reply via email to