[for-4.14,3/3] xen/x86: atomic: Don't allow to write atomically in a pointer to const
diff mbox series

Message ID 20200502160700.19573-4-julien@xen.org
State New
Headers show
Series
  • Rework {read, write}_atomic()
Related show

Commit Message

Julien Grall May 2, 2020, 4:07 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

At the moment, write_atomic() will happily write to a pointer to const.
While there are no use in Xen, it would be best to catch them at
compilation time.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/include/asm-x86/atomic.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jan Beulich May 4, 2020, 10:47 a.m. UTC | #1
On 02.05.2020 18:07, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> At the moment, write_atomic() will happily write to a pointer to const.
> While there are no use in Xen, it would be best to catch them at
> compilation time.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
albeit ...

> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -63,6 +63,8 @@ void __bad_atomic_size(void);
>  
>  #define write_atomic(p, x) ({                             \
>      typeof(*(p)) __x = (x);                               \
> +    /* Check that the pointer is not const */             \
> +    void *__maybe_unused p_ = &__x;                       \

... along the lines of the similar case with guest handles I'd
like to suggest for the comment to be more precise: It's not
the pointer's const-ness you're after, but the pointed to
object's. Maybe "Check that the pointer is not to a const
type" or even just "Check that the pointer is not to const"?

Jan
Julien Grall May 4, 2020, 11:03 a.m. UTC | #2
Hi Jan,

On 04/05/2020 11:47, Jan Beulich wrote:
> On 02.05.2020 18:07, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> At the moment, write_atomic() will happily write to a pointer to const.
>> While there are no use in Xen, it would be best to catch them at
>> compilation time.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
> 
>> --- a/xen/include/asm-x86/atomic.h
>> +++ b/xen/include/asm-x86/atomic.h
>> @@ -63,6 +63,8 @@ void __bad_atomic_size(void);
>>   
>>   #define write_atomic(p, x) ({                             \
>>       typeof(*(p)) __x = (x);                               \
>> +    /* Check that the pointer is not const */             \
>> +    void *__maybe_unused p_ = &__x;                       \
> 
> ... along the lines of the similar case with guest handles I'd
> like to suggest for the comment to be more precise: It's not
> the pointer's const-ness you're after, but the pointed to
> object's. Maybe "Check that the pointer is not to a const
> type" or even just "Check that the pointer is not to const"?

I am happy with "Check that the pointer is not to a const type".

Cheers,

Patch
diff mbox series

diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 6b40f9c9f872..0a332b1fae18 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -63,6 +63,8 @@  void __bad_atomic_size(void);
 
 #define write_atomic(p, x) ({                             \
     typeof(*(p)) __x = (x);                               \
+    /* Check that the pointer is not const */             \
+    void *__maybe_unused p_ = &__x;                       \
     unsigned long x_ = (unsigned long)__x;                \
     switch ( sizeof(*(p)) ) {                             \
     case 1: write_u8_atomic((uint8_t *)(p), x_); break;   \