xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout()
diff mbox series

Message ID 20200730170721.23393-1-julien@xen.org
State New
Headers show
Series
  • xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout()
Related show

Commit Message

Julien Grall July 30, 2020, 5:07 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

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@amazon.com>
---
 xen/include/asm-arm/arm32/cmpxchg.h | 8 +++++++-
 xen/include/asm-arm/arm64/cmpxchg.h | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

Comments

Stefano Stabellini July 30, 2020, 7:44 p.m. UTC | #1
On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> 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@amazon.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  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
>
Bertrand Marquis July 31, 2020, 12:50 p.m. UTC | #2
> On 30 Jul 2020, at 19:07, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> 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@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

> ---
> 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
> 
>

Patch
diff mbox series

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) \