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

From: Dan Williams <dan.j.williams@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	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>
Subject: Re: [PATCH] kernel: sys: fix potential Spectre v1
Date: Wed, 23 May 2018 06:55:50 -0700	[thread overview]
Message-ID: <CAPcyv4jOpZTvvRgx6N9_hJNvpdt+ABpnbMExsGfSDxGpMr5VuA@mail.gmail.com> (raw)
In-Reply-To: <20180523090840.GU12217@hirez.programming.kicks-ass.net>

On Wed, May 23, 2018 at 2:08 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Sorry for being late to the party..
>
> On Wed, May 23, 2018 at 12:03:57AM -0500, Gustavo A. R. Silva wrote:
>
>> +#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;                                                           \
>> +})
>
> Would not something like:
>
>         bool ret = false;
>
>         ....
>
>         if (*ptr < _s) {
>                 *ptr = array_index_nospec(*ptr, _s);
>                 ret = true;
>         }
>
>         ret;
>
> be more obvious?

Yes, that looks even better to me.