linux-kernel - Re: [RFC][PATCH 08/12] s390: Replace cmpxchg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Y7/rMJ75lW7z9PAb@li-4a3a4a4c-28e5-11b2-a85c-a8d192c6f089.ibm.com>
Date:   Thu, 12 Jan 2023 12:12:48 +0100
From:   Alexander Gordeev <agordeev@...ux.ibm.com>
To:     Heiko Carstens <hca@...ux.ibm.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Thomas Richter <tmricht@...ux.ibm.com>,
        torvalds@...ux-foundation.org, corbet@....net, will@...nel.org,
        boqun.feng@...il.com, mark.rutland@....com,
        catalin.marinas@....com, dennis@...nel.org, tj@...nel.org,
        cl@...ux.com, gor@...ux.ibm.com, borntraeger@...ux.ibm.com,
        svens@...ux.ibm.com, Herbert Xu <herbert@...dor.apana.org.au>,
        davem@...emloft.net, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, joro@...tes.org, suravee.suthikulpanit@....com,
        robin.murphy@....com, dwmw2@...radead.org,
        baolu.lu@...ux.intel.com, Arnd Bergmann <arnd@...db.de>,
        penberg@...nel.org, rientjes@...gle.com, iamjoonsoo.kim@....com,
        Andrew Morton <akpm@...ux-foundation.org>, vbabka@...e.cz,
        roman.gushchin@...ux.dev, 42.hyeyoo@...il.com,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, linux-s390@...r.kernel.org,
        linux-crypto@...r.kernel.org, iommu@...ts.linux.dev,
        linux-arch@...r.kernel.org
Subject: Re: [RFC][PATCH 08/12] s390: Replace cmpxchg_double() with
 cmpxchg128()

On Tue, Jan 10, 2023 at 12:46:44PM +0100, Heiko Carstens wrote:
> > > +	/* READ_ONCE() 16 byte header */
> > > +	prev.val = __cdsg(&te->header.val, 0, 0);
> > >  	do {
> > > +		old.val = prev.val;
> > > +		new.val = prev.val;
> > > +		*overflow = old.overflow;

I guess, it would also make sense to place write to overflow 
after the while loop. So the output variable left intact in
case the function bailed out. Not sure if it should be part
of this patch though.

> > > +		if (old.f) {
> > >  			/*
> > >  			 * SDB is already set by hardware.
> > >  			 * Abort and try to set somewhere
> > > @@ -1490,10 +1509,10 @@ static bool aux_set_alert(struct aux_buffer *aux, unsigned long alert_index,
> > >  			 */
> > >  			return false;
> > >  		}
> > > +		new.a = 1;
> > > +		new.overflow = 0;
> > > +		prev.val = __cdsg(&te->header.val, old.val, new.val);
> > > +	} while (prev.val != old.val);
> > 
> > And while this case has an early exit, it only cares about a single bit
> > (although you made it a full word) and so also shouldn't care. If
> > aux_reset_buffer() returns false, @overflow isn't consumed.
> 
> Yes, except that it is anything but obvious that @overflow isn't consumed.