Re: [RFC PATCH 07/14] khwasan: add tag related helper functions

[Dmitry Vyukov <dvyukov@google.com>]

On Wed, Mar 7, 2018 at 7:16 PM, Christopher Lameter <cl@linux.com> 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.