Messages in this thread | | | Date | Wed, 23 May 2018 17:31:18 +0100 | From | Mark Rutland <> | Subject | Re: [PATCH] kernel: sys: fix potential Spectre v1 |
| |
On Wed, May 23, 2018 at 04:07:37PM +0100, Mark Rutland wrote: > 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 see GCC do this at -O0, but so far I haven't tricked it into doing this at -O1 or above.
Regardless, I worry this is fragile -- GCC *can* generate code as per the above, even if it's unlikely to.
> 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. :/
No luck so far, but I'll keeep fighting...
GCC will happily pull a common suffix after the branch, e.g.
if (cond) { foo(); bar(); } else { bar(); }
.. goes to:
if (cond) foo()
bar();
... but I can't convince it to pull a common prefix before the branch.
Mark.
| |