From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f200.google.com (mail-pf0-f200.google.com [209.85.192.200]) by kanga.kvack.org (Postfix) with ESMTP id A8EE46B0003 for ; Thu, 8 Mar 2018 04:10:07 -0500 (EST) Received: by mail-pf0-f200.google.com with SMTP id e10so2725820pff.3 for ; Thu, 08 Mar 2018 01:10:07 -0800 (PST) Received: from mail-sor-f41.google.com (mail-sor-f41.google.com. [209.85.220.41]) by mx.google.com with SMTPS id r82sor4603388pfg.60.2018.03.08.01.10.06 for (Google Transport Security); Thu, 08 Mar 2018 01:10:06 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <226055ec7c1a01dd8211ca9a8b34c07162be37fa.1520017438.git.andreyknvl@google.com> <20180305143246.o7bass2rhbksneqb@lakrids.cambridge.arm.com> From: Dmitry Vyukov Date: Thu, 8 Mar 2018 10:09:43 +0100 Message-ID: Subject: Re: [RFC PATCH 07/14] khwasan: add tag related helper functions Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-mm@kvack.org List-ID: To: Christopher Lameter Cc: Andrey Konovalov , Mark Rutland , Andrey Ryabinin , Alexander Potapenko , Jonathan Corbet , Catalin Marinas , Will Deacon , Theodore Ts'o , Jan Kara , Christopher Li , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Masahiro Yamada , Michal Marek , Ard Biesheuvel , Yury Norov , Nick Desaulniers , Marc Zyngier , Bob Picco , Suzuki K Poulose , Kristina Martsenko , Punit Agrawal , Dave Martin , James Morse , Julien Thierry , Michael Weiser , Steve Capper , Ingo Molnar , Thomas Gleixner , Sandipan Das , Paul Lawrence , David Woodhouse , Kees Cook , Geert Uytterhoeven , Josh Poimboeuf , Arnd Bergmann , kasan-dev , linux-doc@vger.kernel.org, LKML , Linux ARM , linux-ext4@vger.kernel.org, linux-sparse@vger.kernel.org, Linux Memory Management List , Linux Kbuild mailing list , Kostya Serebryany , Evgeniy Stepanov , Lee Smith , Ramana Radhakrishnan , Jacob Bramley , Ruben Ayrapetyan , Kees Cook , Jann Horn , Mark Brand On Wed, Mar 7, 2018 at 7:16 PM, Christopher Lameter 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.