lkml.org 
[lkml]   [2018]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] kernel: sys: fix potential Spectre v1
On Wed, May 23, 2018 at 11:08:40AM +0200, Peter Zijlstra wrote:
>
> Sorry for being late to the party..

Likewise!

> 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?

I think that either way, we have a potential problem if the compiler
generates a branch dependent on the result of validate_index_nospec().

In that case, we could end up with codegen approximating:

bool safe = false;

if (idx < bound) {
idx = array_index_nospec(idx, bound);
safe = true;
}

// this branch can be mispredicted
if (safe) {
foo = array[idx];
}

... and thus we lose the nospec protection.

I also suspect that compiler transformations mean that this might
already be the case for patterns like:

if (idx < bound) {
safe_idx = array_index_nospec(idx, bound)];
...
foo = array[safe_idx];
}

... if the compiler can transform that to something like:

if (idx < bound) {
idx = array_index_nospec(idx, bound);
}

// can be mispredicted
if (idx < bound) {
foo = array[idx];
}

... which I think a compiler might be capable of, depending on the rest
of the function body (e.g. if there's a common portion shared with the
else case).

I'll see if I can trigger that in a test case. :/

Thanks,
Mark.

\
 
 \ /
  Last update: 2018-05-23 17:08    [W:0.168 / U:0.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site