On 05/01/18 17:24, Jeff Law wrote: > On 01/04/2018 06:58 AM, Richard Earnshaw wrote: >> >> Recently, Google Project Zero disclosed several classes of attack >> against speculative execution. One of these, known as variant-1 >> (CVE-2017-5753), allows explicit bounds checks to be bypassed under >> speculation, providing an arbitrary read gadget. Further details can >> be found on the GPZ blog [1] and the documentation that is included >> with the first patch. > So I think it's important for anyone reading this stuff to read > Richard's paragraph carefully -- "an arbitrary read gadget". > > I fully expect that over the course of time we're going to see other > arbitrary read gadgets to be found. Those gadgets may have lower > bandwidth, have a higher degree of jitter or be harder to exploit, but > they're out there just waiting to be discovered. > > So I don't expect this to be the only mitigation we have to put into place. > > Anyway... > > >> >> Some optimizations are permitted to make the builtin easier to use. >> The final two arguments can both be omitted (c++ style): failval will >> default to 0 in this case and if cmpptr is omitted ptr will be used >> for expansions of the range check. In addition either lower or upper >> (but not both) may be a literal NULL and the expansion will then >> ignore that boundary condition when expanding. > So what are the cases where FAILVAL is useful rather than just using > zero (or some other constant) all the time?
So some background first. My initial attempts to define a builtin were based entirely around trying to specify the builtin without out any hard functional behaviour as well. The specification was a mess. Things just became so much easier to specify when I moved to defining a logical behaviour on top of the intended side effects on speculation. Having done that it seemed sensible to permit the user to use the builtin in more creative ways by allowing it to select the failure value. The idea was that code such as if (ptr >= base && ptr < limit) // bounds check return *ptr; return FAILVAL; could simply be rewritten as return __builtin_load_no_speculate (ptr, base, limit, FAILVAL); and now you don't have to worry about writing the condition out twice or any other such nonsense. In this case the builtin does exactly what you want it to do. (It's also easier now to write test cases that check the correctness of the builtin's expansion, since you can test for a behaviour of the code's execution, even if you can't check the speculation behaviour properly.) > > Similarly under what conditions would one want to use CMPPTR rather than > PTR? This was at the request of some kernel folk. They have some cases where CMPPTR may be a pointer to an atomic type and want to do something like if (cmpptr >= lower && cmpptr < upper) val = __atomic_read_and_inc (cmpptr); The atomics are complicated enough already and rewriting them all to additionally inhibit speculation based on the result would be a nightmare. By separating out cmpptr from ptr you can now write if (cmpptr >= lower && cmpptr < upper) { TYPE tmp_val = __atomic_read_and_inc (cmpptr); val = builtin_load_no_speculate (&tmp_val, lower, upper, 0, cmpptr); } It's meaningless in this case to check the bounds of tmp_val - it's just a value pushed onto the stack in order to satisfy the requirement that we need a load that is being speculatively executed; but we can still test against the speculation conditions, which are still the range check for cmpptr. There may be other similar cases as well where you have an object that you want to clean up that is somehow derived from cmpptr but is not cmpptr itself. You do have to be more careful with the extended form, since it is possible to write sequences that don't inhibit speculation in the way you might think they do, but with greater flexibility also comes greater risk. I don't think there's ever a problem if ptr is somehow derived from dereferencing cmpptr. R. > > I wandered down through the LKML thread but didn't see anything which > would shed light on those two questions. > > jeff >>