On Thu, Apr 2, 2020 at 1:55 AM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Wed, Apr 1, 2020 at 4:51 PM Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > It's literally testing a sequence counter for equality. If you get > > tearing in the high bits on the write (or the read), you'd still need > > to have the low bits turn around 4G times to get a matching value. > > Put another way: first you'd have to work however many weeks to do 4 > billion execve() calls, and then you need to hit basically a > single-instruction race to take advantage of it. > > Good luck with that. If you have that kind of God-like capability, > whoever you're attacking stands no chance in the first place. I'm not really worried about someone being able to hit this in practice, especially given that the only thing it lets you do is send signals; I just think that the code is wrong in principle, and that we should avoid having "technically wrong, but works in practice" code in the kernel. This kind of intentional race might also trip up testing tools like the upcoming KCSAN instrumentation, unless it is specifically annotated as an intentionally racing instruction. (For now, KCSAN is 64-bit only, so it won't actually be able to detect this though; and the current KCSAN development branch actually incorrectly considers WRITE_ONCE() to always be atomic; but still, in principle it's the kind of issue KCSAN is supposed to detect, I think.) But even without KCSAN, if we have tearing stores racing with loads, I think that we ought to *at least* have a comment noting that we're intentionally doing that so that people don't copy this kind of code to a place where the high bits change more frequently and correctness matters more. Since the read is already protected by the tasklist_lock, an alternative might be to let the execve path also take that lock to protect the sequence number update, given that execve is not a particularly hot path. Or we could hand-code the equality check and increment operations to be properly race-free.
