lkml.org 
[lkml]   [2018]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 06/19] asm-generic/barrier: mask speculative execution flows
    On Thu, Jan 11, 2018 at 04:46:56PM -0800, Dan Williams wrote:
    > diff --git a/include/linux/nospec.h b/include/linux/nospec.h
    > new file mode 100644
    > index 000000000000..5c66fc30f919
    > --- /dev/null
    > +++ b/include/linux/nospec.h
    > @@ -0,0 +1,71 @@
    > +// SPDX-License-Identifier: GPL-2.0
    > +// Copyright(c) 2018 Intel Corporation. All rights reserved.
    > +
    > +#ifndef __NOSPEC_H__
    > +#define __NOSPEC_H__
    > +
    > +#include <linux/jump_label.h>
    > +#include <asm/barrier.h>
    > +
    > +#ifndef array_ptr_mask
    > +#define array_ptr_mask(idx, sz) \
    > +({ \
    > + unsigned long mask; \
    > + unsigned long _i = (idx); \
    > + unsigned long _s = (sz); \
    > + \
    > + mask = ~(long)(_i | (_s - 1 - _i)) >> (BITS_PER_LONG - 1); \
    > + mask; \
    > +})
    > +#endif
    > +
    > +/**
    > + * __array_ptr - Generate a pointer to an array element, ensuring
    > + * the pointer is bounded under speculation to NULL.
    > + *
    > + * @base: the base of the array
    > + * @idx: the index of the element, must be less than LONG_MAX
    > + * @sz: the number of elements in the array, must be less than LONG_MAX
    > + *
    > + * If @idx falls in the interval [0, @sz), returns the pointer to
    > + * @arr[@idx], otherwise returns NULL.
    > + */
    > +#define __array_ptr(base, idx, sz) \
    > +({ \
    > + union { typeof(*(base)) *_ptr; unsigned long _bit; } __u; \
    > + typeof(*(base)) *_arr = (base); \
    > + unsigned long _i = (idx); \
    > + unsigned long _mask = array_ptr_mask(_i, (sz)); \
    > + \
    > + __u._ptr = _arr + (_i & _mask); \
    > + __u._bit &= _mask; \
    > + __u._ptr; \
    > +})
    > +
    > +#ifdef CONFIG_SPECTRE1_IFENCE
    > +DECLARE_STATIC_KEY_TRUE(nospec_key);
    > +#else
    > +DECLARE_STATIC_KEY_FALSE(nospec_key);
    > +#endif
    > +
    > +#ifdef ifence_array_ptr
    > +/*
    > + * The expectation is that no compiler or cpu will mishandle __array_ptr
    > + * leading to problematic speculative execution. Bypass the ifence
    > + * based implementation by default.
    > + */
    > +#define array_ptr(base, idx, sz) \
    > +({ \
    > + typeof(*(base)) *__ret; \
    > + \
    > + if (static_branch_unlikely(&nospec_key)) \
    > + __ret = ifence_array_ptr(base, idx, sz); \
    > + else \
    > + __ret = __array_ptr(base, idx, sz); \
    > + __ret; \
    > +})


    So I think this wants:

    #ifndef HAVE_JUMP_LABEL
    #error Compiler lacks asm-goto, can generate unsafe code
    #endif

    Suppose the generic array_ptr_mask() is unsafe on some arch and they
    only implement ifence_array_ptr() and they compile without asm-goto,
    then the above reverts to a dynamic condition, which can be speculated.
    If we then speculate into the 'bad' __array_ptr we're screwed.

    > +#else
    > +#define array_ptr __array_ptr
    > +#endif
    > +
    > +#endif /* __NOSPEC_H__ */


    In general I think I would write all this in a form like:

    #define __array_ptr(base, idx, sz) \
    ({ \
    union { typeof(*(base)) *_ptr; unsigned long _bit; } __u; \
    typeof(*(base)) *_arr = (base); \
    unsigned long _i = (idx); \
    unsigned long _mask = array_ptr_mask(_i, (sz)); \
    \
    __u._ptr = _arr + (_i & _mask); \
    __u._bit &= _mask; \
    __u._ptr; \
    })

    #if defined(array_ptr_mask) && defined(ifence_array_ptr)

    #ifndef HAVE_JUMP_LABEL
    #error Compiler lacks asm-goto, can generate unsafe code
    #endif

    #define array_ptr(base, idx, sz) \
    ({ \
    typeof(*(base)) *__ret; \
    \
    if (static_branch_unlikely(&nospec_key)) \
    __ret = ifence_array_ptr(base, idx, sz); \
    else \
    __ret = __array_ptr(base, idx, sz); \
    __ret; \
    })

    #elif defined(array_ptr_mask)

    #define array_ptr(base, idx, sz) __array_ptr(base, idx, sz)

    #elif defined(ifence_array_ptr)

    #define array_ptr(base, idx, sz) ifence_array_ptr(base, idx, sz)

    #else

    /* XXX we want a suitable warning here ? */

    #define array_ptr(base, idx, sz) (idx < sz ? base + idx : NULL)

    #endif

    and stick the generic array_ptr_mask into asm-generic/nospec.h or
    something.

    Then the static key stuff is limited to architectures that define _both_
    array_ptr_mask and ifence_array_ptr.

    \
     
     \ /
      Last update: 2018-01-14 23:25    [W:4.060 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site