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:22:42 -0500	[thread overview]
Message-ID: <9d6eaa24-2872-c93c-81b7-01b6a3ced67c@embeddedor.com> (raw)
In-Reply-To: <CAPcyv4jJ6g7nOAv4hV9RtduiDmdK=MPuE1tzybVROmH3S6994A@mail.gmail.com>

On 05/23/2018 12:15 AM, Dan Williams wrote:
>>
>> 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)                                              \

I'll change the line above by this one:

if (*ptr >= _s)

>> +               ret = false;                                           \
>> +                                                                      \
>> +       *ptr = array_index_nospec(*ptr, _s);                           \
>> +                                                                      \
>> +       ret;                                                           \
>>
>> +})
>>   #endif /* _LINUX_NOSPEC_H */
> 
> Assuming the assembly generation is comparable with the open coded
> version, this looks ok to me.
> 

OK. I'll send a proper patch tomorrow morning.

Thanks for the feedback, Dan.
--
Gustavo