[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen: Port the array_index_nospec() infrastructure from Linux



Hi Andrew,

On 06/07/18 13:37, Andrew Cooper wrote:
> This is as the infrastructure appeared in Linux 4.17, adapted slightly for
> Xen.  The architecture independent array_index_mask_nospec() is ported for
> ARMs benefit, as it currently lacks the optimised version.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> 
> Julien/Stefano: The ARM side of things was beyond my expertise to port to Xen.
> A different option would be avoid porting the generic implementation would
> result in a hard compiler error if someone tried to use array_index_nospec()
> in ARM code.

Below the Arm side of the function. Feel free to integrate that in your patch.

Cheers,

diff --git a/xen/include/asm-arm/arm32/system.h 
b/xen/include/asm-arm/arm32/system.h
index c617b40438..5e1e61d9f0 100644
--- a/xen/include/asm-arm/arm32/system.h
+++ b/xen/include/asm-arm/arm32/system.h
@@ -48,6 +48,25 @@ static inline int local_fiq_is_enabled(void)
     return !(flags & PSR_FIQ_MASK);
 }
 
+#define CSDB    ".inst  0xe320f014"
+
+static inline unsigned long array_index_mask_nospec(unsigned long idx,
+                                                   unsigned long sz)
+ {
+       unsigned long mask;
+
+       asm volatile(
+               "cmp    %1, %2\n"
+       "       sbc     %0, %1, %1\n"
+       CSDB
+       : "=r" (mask)
+       : "r" (idx), "Ir" (sz)
+       : "cc");
+
+       return mask;
+}
+#define array_index_mask_nospec array_index_mask_nospec
+
 #endif
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/arm64/system.h 
b/xen/include/asm-arm/arm64/system.h
index 2e2ee212a1..4cdcba9c87 100644
--- a/xen/include/asm-arm/arm64/system.h
+++ b/xen/include/asm-arm/arm64/system.h
@@ -58,6 +58,28 @@ static inline int local_fiq_is_enabled(void)
     return !(flags & PSR_FIQ_MASK);
 }
 
+#define csdb()  asm volatile("hint #20" : : : "memory")
+
+/*
+ * Generate a mask for array_index__nospec() that is ~0UL when 0 <= idx < sz
+ * and 0 otherwise.
+ */
+static inline unsigned long array_index_mask_nospec(unsigned long idx,
+                                                    unsigned long sz)
+{
+    unsigned long mask;
+
+    asm volatile("     cmp     %1, %2\n"
+                 "     sbc     %0, xzr, xzr\n"
+                 : "=r" (mask)
+                 : "r" (idx), "Ir" (sz)
+                 : "cc");
+    csdb();
+
+    return mask;
+}
+#define array_index_mask_nospec array_index_mask_nospec
+
 #endif
 /*
  * Local variables:


> ---
>   xen/include/asm-x86/system.h | 24 +++++++++++++++
>   xen/include/xen/compiler.h   |  4 +++
>   xen/include/xen/nospec.h     | 70 
> ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 98 insertions(+)
>   create mode 100644 xen/include/xen/nospec.h
> 
> diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
> index 43fb6fe..483cd20 100644
> --- a/xen/include/asm-x86/system.h
> +++ b/xen/include/asm-x86/system.h
> @@ -221,6 +221,30 @@ static always_inline unsigned long __xadd(
>   #define set_mb(var, value) do { xchg(&var, value); } while (0)
>   #define set_wmb(var, value) do { var = value; smp_wmb(); } while (0)
>   
> +/**
> + * array_index_mask_nospec() - generate a mask that is ~0UL when the
> + *      bounds check succeeds and 0 otherwise
> + * @index: array element index
> + * @size: number of elements in array
> + *
> + * Returns:
> + *     0 - (index < size)
> + */
> +static inline unsigned long array_index_mask_nospec(unsigned long index,
> +                                                    unsigned long size)
> +{
> +    unsigned long mask;
> +
> +    asm volatile ( "cmp %[size], %[index]; sbb %[mask], %[mask];"
> +                   : [mask] "=r" (mask)
> +                   : [size] "g" (size), [index] "r" (index) );
> +
> +    return mask;
> +}
> +
> +/* Override default implementation in nospec.h. */
> +#define array_index_mask_nospec array_index_mask_nospec
> +
>   #define local_irq_disable()     asm volatile ( "cli" : : : "memory" )
>   #define local_irq_enable()      asm volatile ( "sti" : : : "memory" )
>   
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 533a8ea..887500e 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -81,6 +81,10 @@
>   #pragma GCC visibility push(hidden)
>   #endif
>   
> +/* Make the optimizer believe the variable can be manipulated arbitrarily. */
> +#define OPTIMIZER_HIDE_VAR(var)                 \
> +    __asm__ ("" : "=g" (var) : "0" (var))
> +
>   /* This macro obfuscates arithmetic on a variable address so that gcc
>      shouldn't recognize the original var, and make assumptions about it */
>   /*
> diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
> new file mode 100644
> index 0000000..4879399
> --- /dev/null
> +++ b/xen/include/xen/nospec.h
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2018 Linus Torvalds. All rights reserved. */
> +/* Copyright(c) 2018 Alexei Starovoitov. All rights reserved. */
> +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
> +/* Copyright(c) 2018 Citrix Systems R&D Ltd. All rights reserved. */
> +
> +#ifndef XEN_NOSPEC_H
> +#define XEN_NOSPEC_H
> +
> +#include <asm/system.h>
> +
> +/**
> + * array_index_mask_nospec() - generate a ~0 mask when index < size, 0 
> otherwise
> + * @index: array element index
> + * @size: number of elements in array
> + *
> + * When @index is out of bounds (@index >= @size), the sign bit will be
> + * set.  Extend the sign bit to all bits and invert, giving a result of
> + * zero for an out of bounds index, or ~0 if within bounds [0, @size).
> + */
> +#ifndef array_index_mask_nospec
> +static inline unsigned long array_index_mask_nospec(unsigned long index,
> +                                                    unsigned long size)
> +{
> +    /*
> +     * Always calculate and emit the mask even if the compiler
> +     * thinks the mask is not needed. The compiler does not take
> +     * into account the value of @index under speculation.
> +     */
> +    OPTIMIZER_HIDE_VAR(index);
> +    return ~(long)(index | (size - 1UL - index)) >> (BITS_PER_LONG - 1);
> +}
> +#endif
> +
> +/*
> + * array_index_nospec - sanitize an array index after a bounds check
> + *
> + * For a code sequence like:
> + *
> + *     if (index < size) {
> + *         index = array_index_nospec(index, size);
> + *         val = array[index];
> + *     }
> + *
> + * ...if the CPU speculates past the bounds check then
> + * array_index_nospec() will clamp the index within the range of [0,
> + * size).
> + */
> +#define array_index_nospec(index, size)                                 \
> +({                                                                      \
> +    typeof(index) _i = (index);                                         \
> +    typeof(size) _s = (size);                                           \
> +    unsigned long _mask = array_index_mask_nospec(_i, _s);              \
> +                                                                        \
> +    BUILD_BUG_ON(sizeof(_i) > sizeof(long));                            \
> +    BUILD_BUG_ON(sizeof(_s) > sizeof(long));                            \
> +                                                                        \
> +    (typeof(_i)) (_i & _mask);                                          \
> +})
> +
> +#endif /* XEN_NOSPEC_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.