lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Sep 2020 09:24:43 +0800 From: Boqun Feng <boqun.feng@...il.com> To: Marco Elver <elver@...gle.com> Cc: paulmck@...nel.org, linux-kernel@...r.kernel.org, kasan-dev@...glegroups.com, kernel-team@...com, mingo@...nel.org, andreyknvl@...gle.com, glider@...gle.com, dvyukov@...gle.com, cai@....pw, Will Deacon <will@...nel.org>, Arnd Bergmann <arnd@...db.de>, Daniel Axtens <dja@...ens.net>, Michael Ellerman <mpe@...erman.id.au>, linux-arch@...r.kernel.org Subject: Re: [PATCH kcsan 18/19] bitops, kcsan: Partially revert instrumentation for non-atomic bitops On Wed, Sep 02, 2020 at 08:13:15AM +0200, Marco Elver wrote: > On Wed, Sep 02, 2020 at 11:30AM +0800, Boqun Feng wrote: > > Hi Paul and Marco, > > > > The whole update patchset looks good to me, just one question out of > > curiosity fo this one, please see below: > > > > On Mon, Aug 31, 2020 at 11:18:04AM -0700, paulmck@...nel.org wrote: > > > From: Marco Elver <elver@...gle.com> > > > > > > Previous to the change to distinguish read-write accesses, when > > > CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=y is set, KCSAN would consider > > > the non-atomic bitops as atomic. We want to partially revert to this > > > behaviour, but with one important distinction: report racing > > > modifications, since lost bits due to non-atomicity are certainly > > > possible. > > > > > > Given the operations here only modify a single bit, assuming > > > non-atomicity of the writer is sufficient may be reasonable for certain > > > usage (and follows the permissible nature of the "assume plain writes > > > atomic" rule). In other words: > > > > > > 1. We want non-atomic read-modify-write races to be reported; > > > this is accomplished by kcsan_check_read(), where any > > > concurrent write (atomic or not) will generate a report. > > > > > > 2. We do not want to report races with marked readers, but -do- > > > want to report races with unmarked readers; this is > > > accomplished by the instrument_write() ("assume atomic > > > write" with Kconfig option set). > > > > > > > Is there any code in kernel using the above assumption (i.e. > > non-atomicity of the writer is sufficient)? IOW, have you observed > > anything bad (e.g. an anoying false positive) after applying the > > read_write changes but without this patch? > > We were looking for an answer to: > > https://lkml.kernel.org/r/20200810124516.GM17456@casper.infradead.org > > Initially we thought using atomic bitops might be required, but after a > longer offline discussion realized that simply marking the reader in > this case, but retaining the non-atomic bitop is probably all that's > needed. > > The version of KCSAN that found the above was still using KCSAN from > Linux 5.8, but we realized with the changed read-write instrumentation > to bitops in this series, we'd regress and still report the race even if > the reader was marked. To avoid this with the default KCSAN config, we > determined that we need the patch here. > Thanks for the background! Now I see the point of having this patch ;-) FWIW, feel free to add for the whole series: Reviewed-by: Boqun Feng <boqun.feng@...il.com> Regards, Boqun > The bitops are indeed a bit more special, because for both the atomic > and non-atomic bitops we *can* reason about the generated code (since we > control it, although not sure about the asm-generic ones), and that > makes reasoning about accesses racing with non-atomic bitops more > feasible. At least that's our rationale for deciding that reverting > non-atomic bitops treatment to it's more relaxed version is ok. > > Thanks, > -- Marco
Powered by blists - more mailing lists