Re: [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout()

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

> On 30 Jul 2020, at 19:07, Julien Grall <julien@xxxxxxx> wrote:
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> The function __cmpxchg_mb_timeout() was intended to have the same
> semantics as __cmpxchg_mb(). Unfortunately, the memory barriers were
> not added when first implemented.
> 
> There is no known issue with the existing callers, but the barriers are
> added given this is the expected semantics in Xen.
> 
> The issue was introduced by XSA-295.
> 
> Backport: 4.8+
> Fixes: 86b0bc958373 ("xen/arm: cmpxchg: Provide a new helper that can 
> timeout")
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

> ---
> xen/include/asm-arm/arm32/cmpxchg.h | 8 +++++++-
> xen/include/asm-arm/arm64/cmpxchg.h | 8 +++++++-
> 2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/arm32/cmpxchg.h 
> b/xen/include/asm-arm/arm32/cmpxchg.h
> index 49ca2a0d7ab1..0770f272ee99 100644
> --- a/xen/include/asm-arm/arm32/cmpxchg.h
> +++ b/xen/include/asm-arm/arm32/cmpxchg.h
> @@ -147,7 +147,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile 
> void *ptr,
>                                              int size,
>                                              unsigned int max_try)
> {
> -     return __int_cmpxchg(ptr, old, new, size, true, max_try);
> +     bool ret;
> +
> +     smp_mb();
> +     ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
> +     smp_mb();
> +
> +     return ret;
> }
> 
> #define cmpxchg(ptr,o,n)                                              \
> diff --git a/xen/include/asm-arm/arm64/cmpxchg.h 
> b/xen/include/asm-arm/arm64/cmpxchg.h
> index 5bc2e1f78674..fc5c60f0bd74 100644
> --- a/xen/include/asm-arm/arm64/cmpxchg.h
> +++ b/xen/include/asm-arm/arm64/cmpxchg.h
> @@ -160,7 +160,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile 
> void *ptr,
>                                              int size,
>                                              unsigned int max_try)
> {
> -     return __int_cmpxchg(ptr, old, new, size, true, max_try);
> +     bool ret;
> +
> +     smp_mb();
> +     ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
> +     smp_mb();
> +
> +     return ret;
> }
> 
> #define cmpxchg(ptr, o, n) \
> -- 
> 2.17.1
> 
>