Date:   Mon,  7 Jun 2021 14:56:46 +0200
From:   Marco Elver <elver@...gle.com>
To:     elver@...gle.com, paulmck@...nel.org
Cc:     boqun.feng@...il.com, mark.rutland@....com, will@...nel.org,
        glider@...gle.com, dvyukov@...gle.com, kasan-dev@...glegroups.com,
        linux-kernel@...r.kernel.org
Subject: [PATCH 0/7] kcsan: Introduce CONFIG_KCSAN_PERMISSIVE

While investigating a number of data races, we've encountered data-racy
accesses on flags variables to be very common. The typical pattern is a
reader masking all but one bit, and the writer setting/clearing only 1
bit (current->flags being a frequently encountered case; mm/sl[au]b.c
disables KCSAN for this reason currently).

Since these types of "trivial" data races are common (assuming they're
intentional and hard to miscompile!), having the option to filter them
(like we currently do for other types of data races) will avoid forcing
everyone to mark them, and deliberately left to preference at this time.

The primary motivation is to move closer towards more easily filtering
interesting data races (like [1], [2], [3]) on CI systems (e.g. syzbot),
without the churn to mark all such "trivial" data races.
[1] https://lkml.kernel.org/r/20210527092547.2656514-1-elver@google.com
[2] https://lkml.kernel.org/r/20210527104711.2671610-1-elver@google.com
[3] https://lkml.kernel.org/r/20210209112701.3341724-1-elver@google.com

Notably, the need for further built-in filtering has become clearer as
we notice some other CI systems (without active moderation) trying to
employ KCSAN, but usually have to turn it down quickly because their
reports are quickly met with negative feedback:
https://lkml.kernel.org/r/YHSPfiJ/h/f3ky5n@elver.google.com

The rules are implemented and guarded by a new option
CONFIG_KCSAN_PERMISSIVE. With it, we will ignore data races with only
1-bit value changes. Please see more details in in patch 7/7.

The rest of the patches are cleanups and improving configuration.

I ran some experiments to see what data races we're left with. With
CONFIG_KCSAN_PERMISSIVE=y paired with syzbot's current KCSAN config
(minimal kernel, most permissive KCSAN options), we're "just" about ~100
reports away to a pretty silent KCSAN kernel:

  https://github.com/google/ktsan/tree/kcsan-permissive-with-dataraces
  [ !!Disclaimer!! None of the commits are usable patches nor guaranteed
    to be correct -- they merely resolve a data race so it wouldn't be
    shown again and then moved on. Expect that simply marking is not
    enough for some! ]

Most of the data races look interesting enough, and only few already had
a comment nearby explaining what's happening.

All data races on current->flags, and most other flags are absent
(unlike before). Those that were reported all had value changes with >1
bit. A limitation is that few data races are still reported where the
reader is only interested in 1 bit but the writer changed more than 1
bit. A complete approach would require compiler changes in addition to
the changes in this series -- but since that would further reduce the
data races reported, the simpler and conservative approach is to stick
to the value-change based rules for now.

Marco Elver (7):
  kcsan: Improve some Kconfig comments
  kcsan: Remove CONFIG_KCSAN_DEBUG
  kcsan: Introduce CONFIG_KCSAN_STRICT
  kcsan: Reduce get_ctx() uses in kcsan_found_watchpoint()
  kcsan: Rework atomic.h into permissive.h
  kcsan: Print if strict or non-strict during init
  kcsan: permissive: Ignore data-racy 1-bit value changes

 Documentation/dev-tools/kcsan.rst | 12 ++++
 kernel/kcsan/atomic.h             | 23 --------
 kernel/kcsan/core.c               | 77 ++++++++++++++++---------
 kernel/kcsan/kcsan_test.c         | 32 +++++++++++
 kernel/kcsan/permissive.h         | 94 +++++++++++++++++++++++++++++++
 lib/Kconfig.kcsan                 | 39 +++++++++----
 6 files changed, 215 insertions(+), 62 deletions(-)
 delete mode 100644 kernel/kcsan/atomic.h
 create mode 100644 kernel/kcsan/permissive.h

-- 
2.32.0.rc1.229.g3e70b5a671-goog

Powered by blists - more mailing lists