On Wed, 2025-06-25 at 23:43 +0200, Paul Chaignon wrote: [...] > > So, suppose there is a program: > > > > 15: (18) r1 = 0x2020200005642020 > > 17: (7b) *(u64 *)(r10 -264) = r1 > > > > Insn processing sequence would look like (starting from 15): > > - prev_insn_idx <- 15 > > - do_check_insn() > > - env->insn_idx <- 17 > > - prev_insn_idx <- 17 > > - do_check_insn(): > > - nospec_result <- true > > - env->insn_idx <- 18 > > - state->speculative && cur_aux(env)->nospec_result == true: > > - WARN_ON_ONCE(18 != 17 + 1) // no warning > > > > What do I miss? > > In the if condition, "cur_aux(env)" points to the aux data of the next > instruction (#17 here) because we incremented "insn_idx" in > do_check_insn(). In my fix, "insn" points to the previous instruction > because we retrieved it before calling do_check_insn(). > > Therefore, the processing sequence would look like: > - prev_insn_idx <- 15 > - do_check_insn() > - env->insn_idx <- 17 > - state->speculative && cur_aux(env)->nospec_result == true: > - WARN_ON_ONCE(17 != 15 + 1) // warning I'm sorry, I'm a bit slow today (well, maybe not only today). Isn't it a slightly different bug in the original check? Suppose current insn is ST/STX that do_check_insn() marked as nospec_result. I think the intent was to stop branch processing right at that point, as nospec instruction would be inserted after this store => no need to speculate further. In other words, cur_aux(env)->nospec_result pointing to instruction after ST/STX was not anticipated. (Luis, wdyt?) > I added a verbose() and recompiled to confirm those numbers. > > If that makes sense, I'll send a v2 with: > - A better description, probably with a walkthrough. > - A test case simplified from the syzkaller repro. > - insn_sz renamed to prev_insn_sz for clarity. [...]
