Re: [PATCH] kernel: sys: fix potential Spectre v1

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] kernel: sys: fix potential Spectre v1
Date: Wed, 23 May 2018 00:03:57 -0500	[thread overview]
Message-ID: <112349fb-837c-7b91-e256-a1c443710150@embeddedor.com> (raw)
In-Reply-To: <CAPcyv4i6zOJZakNVW=y8NyUFB1EzOxBtoOGVhWCwysuTi6grvg@mail.gmail.com>



On 05/22/2018 03:50 PM, Dan Williams wrote:
>>>
>>> Dan,
>>>
>>> What do you think about this first draft:
>>>
>>> diff --git a/include/linux/nospec.h b/include/linux/nospec.h
>>> index e791ebc..6154183 100644
>>> --- a/include/linux/nospec.h
>>> +++ b/include/linux/nospec.h
>>> @@ -55,4 +55,16 @@ static inline unsigned long
>>> array_index_mask_nospec(unsigned long index,
>>>                                                                          \
>>>           (typeof(_i)) (_i & _mask);                                     \
>>>    })
>>> +
>>> +#define validate_index_nospec(index, size)                            \
>>> +({                                                                    \
>>> +       typeof(index) *ptr = &(index);                                 \
>>> +       typeof(size) _s = (size);                                      \
>>> +                                                                      \
>>> +       BUILD_BUG_ON(sizeof(*ptr) > sizeof(long));                     \
>>> +       BUILD_BUG_ON(sizeof(_s) > sizeof(long));                       \
>>> +                                                                      \
>>> +       *ptr >= _s ? false :                                           \
>>> +       (*ptr = array_index_nospec(*ptr, _s) ? true : true);           \
>>
>>
>> This actually should be:
>>
>> ((*ptr = array_index_nospec(*ptr, _s)) ? true : true);
>>
> 
> Let's not use ternary conditionals at all to make this more readable.
> 

OK. How about this:

diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index e791ebc..498995b 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -55,4 +55,21 @@ static inline unsigned long 
array_index_mask_nospec(unsigned long index,
                                                                        \
         (typeof(_i)) (_i & _mask);                                     \
  })
+
+#define validate_index_nospec(index, size)                            \
+({                                                                    \
+       bool ret = true;                                               \
+       typeof(index) *ptr = &(index);                                 \
+       typeof(size) _s = (size);                                      \
+                                                                      \
+       BUILD_BUG_ON(sizeof(*ptr) > sizeof(long));                     \
+       BUILD_BUG_ON(sizeof(_s) > sizeof(long));                       \
+                                                                      \
+       if (*ptr >= size)                                              \
+               ret = false;                                           \
+                                                                      \
+       *ptr = array_index_nospec(*ptr, _s);                           \
+                                                                      \
+       ret;                                                           \
+})
  #endif /* _LINUX_NOSPEC_H */

Thanks
--
Gustavo
