On Wed, Mar 7, 2018 at 7:16 PM, Christopher Lameter <cl@xxxxxxxxx> wrote: > > On Tue, 6 Mar 2018, Andrey Konovalov wrote: > >> >> + u32 state = this_cpu_read(prng_state); >> >> + >> >> + state = 1664525 * state + 1013904223; >> >> + this_cpu_write(prng_state, state); >> > >> > Have you considered preemption here? Is the assumption that it happens >> > sufficiently rarely that cross-contaminating the prng state isn't a >> > problem? >> >> Hi Mark! >> >> Yes, I have. If a preemption happens between this_cpu_read and >> this_cpu_write, the only side effect is that we'll give a few >> allocated in different contexts objects the same tag. Sine KHWASAN is >> meant to be used a probabilistic bug-detection debug feature, this >> doesn't seem to have serious negative impact. >> >> I'll add a comment about this though. > > You could use this_cpu_cmpxchg here to make it a bit better but it > probably does not matter. Hi, The non-atomic RMW sequence is not just "doesn't seem to have serious negative impact", it in fact has positive effect. Ideally the tags use strong randomness to prevent any attempts to predict them during explicit exploit attempts. But strong randomness is expensive, and we did an intentional trade-off to use a PRNG (may potentially be revised in future, but for now we don't have enough info to do it). In this context, interrupts that randomly skew PRNG at unpredictable points do only good. cmpxchg would also lead to skewing, but non-atomic sequence allows more non-determinism (and maybe a dash less expensive?). This probably deserves a comment, though.
