linux-kernel - Re: [PATCH] LoongArch: Fixup cmpxchg sematic for memory barrier

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: <8573346a40ff1490cdc15d5e11d982a06e98d3bb.camel@xry111.site>
Date:   Tue, 01 Aug 2023 11:10:55 +0800
From:   Xi Ruoyao <xry111@...111.site>
To:     guoren@...nel.org, chenhuacai@...nel.or, kernel@...0n.name,
        arnd@...db.de, andi.shyti@...ux.intel.com, wangrui@...ngson.cn,
        andrzej.hajda@...el.com, peterz@...radead.org, will@...nel.org,
        boqun.feng@...il.com, mark.rutland@....com
Cc:     loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH] LoongArch: Fixup cmpxchg sematic for memory barrier

On Mon, 2023-07-31 at 21:15 -0400, guoren@...nel.org wrote:
> From: Guo Ren <guoren@...ux.alibaba.com>
> 
> When cmpxchg failed, no memory barrier was needed to take. Only
> when cmpxchg success and the new value is written, then the memory
> barriers needed.
> 
>  - cmpxchg_asm:   Unnecessary __WEAK_LLSC_WB for the fail path would
>                   reduce the performance of the cmpxchg loop trying.

I'm not an expert in memory models, but in practice this barrier is
really needed or cmpxchg will be "not atomic".  See
https://lore.kernel.org/linux-mips/1d49da11-51d5-e148-cb02-9bd0ee57fae6@flygoat.com/.

>  - cmpxchg_small: Miss an necessary __WEAK_LLSC_WB when sc succeeds.
>                   It's a bug because there is no memory synchronization
>                   when sc succeeds.
> 
> Signed-off-by: Guo Ren <guoren@...ux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@...nel.org>
> ---
>  arch/loongarch/include/asm/cmpxchg.h | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/loongarch/include/asm/cmpxchg.h b/arch/loongarch/include/asm/cmpxchg.h
> index 979fde61bba8..6a05b92814b6 100644
> --- a/arch/loongarch/include/asm/cmpxchg.h
> +++ b/arch/loongarch/include/asm/cmpxchg.h
> @@ -102,8 +102,8 @@ __arch_xchg(volatile void *ptr, unsigned long x, int size)
>         "       move    $t0, %z4                        \n"             \
>         "       " st "  $t0, %1                         \n"             \
>         "       beqz    $t0, 1b                         \n"             \
> -       "2:                                             \n"             \
>         __WEAK_LLSC_MB                                                  \
> +       "2:                                             \n"             \
>         : "=&r" (__ret), "=ZB"(*m)                                      \
>         : "ZB"(*m), "Jr" (old), "Jr" (new)                              \
>         : "t0", "memory");                                              \
> @@ -148,10 +148,8 @@ static inline unsigned int __cmpxchg_small(volatile void *ptr, unsigned int old,
>         "       or              %1, %1, %z6     \n"
>         "       sc.w            %1, %2          \n"
>         "       beqz            %1, 1b          \n"
> -       "       b               3f              \n"
> -       "2:                                     \n"
>         __WEAK_LLSC_MB
> -       "3:                                     \n"
> +       "2:                                     \n"
>         : "=&r" (old32), "=&r" (temp), "=ZC" (*ptr32)
>         : "ZC" (*ptr32), "Jr" (mask), "Jr" (old), "Jr" (new)
>         : "memory");

-- 
Xi Ruoyao <xry111@...111.site>
School of Aerospace Science and Technology, Xidian University