linux-kernel - Re: [RFC][PATCH 1/5] mips/atomic: Fix cmpxchg64 barriers

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190424210019.knq4nijg3xoqxzhg@pburton-laptop>
Date:   Wed, 24 Apr 2019 21:00:25 +0000
From:   Paul Burton <paul.burton@...s.com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "akiyks@...il.com" <akiyks@...il.com>,
        "andrea.parri@...rulasolutions.com" 
        <andrea.parri@...rulasolutions.com>,
        "boqun.feng@...il.com" <boqun.feng@...il.com>,
        "dlustig@...dia.com" <dlustig@...dia.com>,
        "dhowells@...hat.com" <dhowells@...hat.com>,
        "j.alglave@....ac.uk" <j.alglave@....ac.uk>,
        "luc.maranget@...ia.fr" <luc.maranget@...ia.fr>,
        "npiggin@...il.com" <npiggin@...il.com>,
        "paulmck@...ux.ibm.com" <paulmck@...ux.ibm.com>,
        "will.deacon@....com" <will.deacon@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>
Subject: Re: [RFC][PATCH 1/5] mips/atomic: Fix cmpxchg64 barriers

Hi Peter,

On Wed, Apr 24, 2019 at 02:36:57PM +0200, Peter Zijlstra wrote:
> There were no memory barriers on cmpxchg64() _at_all_. Fix this.

This does looks problematic, but it's worth noting that this code path
is only applicable to 32b kernels running on 64b CPUs which is pretty
rare. The commit message as-is suggests to me that all configurations
are broken, which isn't the case (at least, not in this respect :) ).

> 
> Cc: Paul Burton <paul.burton@...s.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  arch/mips/include/asm/cmpxchg.h |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- a/arch/mips/include/asm/cmpxchg.h
> +++ b/arch/mips/include/asm/cmpxchg.h
> @@ -290,9 +290,11 @@ static inline unsigned long __cmpxchg64(
>  	 * will cause a build error unless cpu_has_64bits is a		\
>  	 * compile-time constant 1.					\
>  	 */								\
> -	if (cpu_has_64bits && kernel_uses_llsc)				\
> +	if (cpu_has_64bits && kernel_uses_llsc) {			\
> +		smp_mb__before_llsc();					\
>  		__res = __cmpxchg64((ptr), __old, __new);		\
> -	else								\
> +		smp_llsc_mb();						\
> +	} else								\
>  		__res = __cmpxchg64_unsupported();			\

It would be good to also add braces around the else block, and I believe
checkpatch should be complaining about that ("braces {} should be used
on all arms of this statement").

Thanks,
    Paul

>  									\
>  	__res;								\
> 
>