On Sat, Dec 22, 2018 at 11:03:31PM -0600, Gustavo A. R. Silva wrote: > Alexei, > > On 12/22/18 10:12 PM, Alexei Starovoitov wrote: > > On Sat, Dec 22, 2018 at 09:37:02PM -0600, Gustavo A. R. Silva wrote: > > > > > > Can't we have the case in which the code can be "trained" to read > > > perfectly valid values for prog->len for quite a while, making the > > > microcode come into place and speculate about: > > > > > > 1013 if (flen == 0 || flen > BPF_MAXINSNS) > > > 1014 return false; > > > > > > and then make flen to be greater than BPF_MAXINSNS? > > > > Yes. The user space can train line 1013 to mispredict by passing > > smaller flen N times and then passing large flen. > > Why do you think it's exploitable? > > > > Without the patch in the mispredicted path the cpu will do > > if (0 < flen) condition and since flen is hot in the cache > > it will happily execute the filter[0] load... > > and about 12-20 u-ops later (depending on u-arch of cpu) when > > branch predictor realizes that it's a miss, the cpu will ignore > > the values computed in the shadow cpu registers used by speculative > > execution > > and go back to the 'return false' execution path. > > The side effect of bringing filter[0] value in L1 cache is still there. > > The cpu is incapable to undo that cache load. That's what spectre1 is about. > > Do you see how filter[0] value in cpu L1 cache is exploitable? > > > > In this regard, the policy has been to kill the speculation on that > first load (as I mentioned in my last email. It is also mentioned in > the commit log for every patch). > > > I took another look at the following patches: > > "net: core: Fix Spectre v1 vulnerability" > > "nfc: af_nfc: Fix Spectre v1 vulnerability" > > "can: af_can: Fix Spectre v1 vulnerability" > > and I have to say that none of them are necessary. > > I'm not sure whether there were other patches that pretend to fix spectre1. > > > > It's not about pretending to fix it. It's about trying to prevent the > conditions that can actually trigger the exploitation of a potential > vulnerability. The code is not always the same, it changes, it evolves, > and we are currently trying to catch that first load (that's what smatch > does in all these cases) that could eventually lead to an actual vuln.
in other words there is no bug and there is no vulnerability, but there is a 'policy' set by ... ? So hence Nack to the policy and Nack to the patches.