[PATCH v2 1/2] kbuild: rust: enable KCSAN support

[PATCH v2 1/2] kbuild: rust: enable KCSAN support

[Marco Elver]

Rust supports ThreadSanitizer instrumentation via LLVM, which is what
KCSAN relies on; however, Rust code is not currently instrumented.

Pass -Zsanitizer=thread and the same TSan backend options used for C
code to rustc. Wire up KCSAN_SANITIZE in scripts/Makefile.lib for parity
with C (though rustc currently rejects linking crates with mismatched
instrumentation).

rustc only accepts -Zsanitizer=thread for targets that declare support
for it. Of the targets used by the kernel, this is only possible for the
generated x86_64 target specification, so restrict instrumentation of
Rust code to x86_64 for now via HAVE_KCSAN_RUSTC. On other architectures
Rust code remains uninstrumented as before.

All TSan options used are supported by the LLVM versions of the minimum
supported rustc, so no rustc-option checks are needed (they would also
fail, because x86_64-unknown-none used for option checks does not
support -Zsanitizer=thread).

Unlike Clang and GCC, an equivalent to -fno-optimize-sibling-calls is
omitted for Rust because rustc provides no corresponding option. While
disabling tail-call optimizations helps preserve complete stacks traces
in reports, it is optional and not required for KCSAN to function.

With this enabled we can detect data races in kernel Rust code and
between Rust and C code.

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Acked-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* Add note on -fno-optimize-sibling-calls to commit message.
---
 Documentation/dev-tools/kcsan.rst |  7 +++++++
 lib/Kconfig.kcsan                 |  7 +++++++
 scripts/Makefile.kcsan            | 13 +++++++++++++
 scripts/Makefile.lib              |  3 +++
 scripts/generate_rust_target.rs   |  2 +-
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/dev-tools/kcsan.rst b/Documentation/dev-tools/kcsan.rst
index 8575178aa87f..3bb8754f844b 100644
--- a/Documentation/dev-tools/kcsan.rst
+++ b/Documentation/dev-tools/kcsan.rst
@@ -14,6 +14,9 @@ Usage
 KCSAN is supported by both GCC and Clang. With GCC we require version 11 or
 later, and with Clang also require version 11 or later.
 
+Rust code is instrumented as well on architectures whose Rust target supports
+it (currently only x86_64; see ``CONFIG_HAVE_KCSAN_RUSTC``).
+
 To enable KCSAN configure the kernel with::
 
     CONFIG_KCSAN = y
@@ -121,6 +124,10 @@ the below options are available:
 
     KCSAN_SANITIZE := n
 
+  Note that ``rustc`` rejects linking crates built with and without KCSAN
+  instrumentation, so ``KCSAN_SANITIZE`` and ``KCSAN_SANITIZE_<target>.o``
+  cannot currently be used for Rust code.
+
 .. _"Marking Shared-Memory Accesses" in the LKMM: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/memory-model/Documentation/access-marking.txt
 
 Furthermore, it is possible to tell KCSAN to show or hide entire classes of
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index 4ce4b0c0109c..f0096e1c8b61 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -10,6 +10,13 @@ config HAVE_KCSAN_COMPILER
 	  For the list of compilers that support KCSAN, please see
 	  <file:Documentation/dev-tools/kcsan.rst>.
 
+config HAVE_KCSAN_RUSTC
+	def_bool RUST && X86_64
+	help
+	  rustc only accepts -Zsanitizer=thread for targets that declare
+	  support for it; of the kernel's Rust targets, only x86_64 does.
+	  Elsewhere, Rust code is not instrumented.
+
 menuconfig KCSAN
 	bool "KCSAN: dynamic data race detector"
 	depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
diff --git a/scripts/Makefile.kcsan b/scripts/Makefile.kcsan
index 19f693b68a96..01af6fd71006 100644
--- a/scripts/Makefile.kcsan
+++ b/scripts/Makefile.kcsan
@@ -24,3 +24,16 @@ kcsan-cflags += $(call cc-option,$(call cc-param,tsan-instrument-func-entry-exit
 endif
 
 export CFLAGS_KCSAN := $(kcsan-cflags)
+
+ifdef CONFIG_HAVE_KCSAN_RUSTC
+# The minimum supported rustc's LLVM supports all of these options.
+rustc-param = -Cllvm-args=-$(1)
+kcsan-rustflags := -Zsanitizer=thread \
+	$(call rustc-param,tsan-compound-read-before-write=1) \
+	$(call rustc-param,tsan-distinguish-volatile=1)
+ifndef CONFIG_KCSAN_WEAK_MEMORY
+kcsan-rustflags += $(call rustc-param,tsan-instrument-func-entry-exit=0)
+endif
+endif
+
+export RUSTFLAGS_KCSAN := $(kcsan-rustflags)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0a4fdd8bd975..78f1c23e54be 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -98,6 +98,9 @@ ifeq ($(CONFIG_KCSAN),y)
 _c_flags += $(if $(patsubst n%,, \
 	$(KCSAN_SANITIZE_$(target-stem).o)$(KCSAN_SANITIZE)$(is-kernel-object)), \
 	$(CFLAGS_KCSAN))
+_rust_flags += $(if $(patsubst n%,, \
+	$(KCSAN_SANITIZE_$(target-stem).o)$(KCSAN_SANITIZE)$(is-kernel-object)), \
+	$(RUSTFLAGS_KCSAN))
 # Some uninstrumented files provide implied barriers required to avoid false
 # positives: set KCSAN_INSTRUMENT_BARRIERS for barrier instrumentation only.
 _c_flags += $(if $(patsubst n%,, \
diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs
index 7687b0dd5474..4b02bffa6b46 100644
--- a/scripts/generate_rust_target.rs
+++ b/scripts/generate_rust_target.rs
@@ -231,7 +231,7 @@ fn main() {
         }
         ts.push("features", features);
         ts.push("llvm-target", "x86_64-linux-gnu");
-        ts.push("supported-sanitizers", ["kcfi", "kernel-address"]);
+        ts.push("supported-sanitizers", ["kcfi", "kernel-address", "thread"]);
         if cfg.rustc_version_atleast(1, 91, 0) {
             ts.push("target-pointer-width", 64);
         } else {
-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v2 2/2] rust: kcsan: add KUnit test for data race detection

[Marco Elver]

Add a minimal KUnit test that checks that KCSAN detects data races in
Rust code: two threads concurrently increment a shared counter without
synchronization, and a probe on the console tracepoint waits for the
KCSAN report (similar to kcsan_test.c):

| BUG: KCSAN: data-race in _RNvNtNtCskctMpSfuwL9_6kernel10kcsan_test5tests8racy_inc / _RNvNtNtCskctMpSfuwL9_6kernel10kcsan_test5tests8racy_inc
|
| read-write to 0xffffffffb751ae28 of 8 bytes by task 116 on cpu 5:
|  _RNvNtNtCskctMpSfuwL9_6kernel10kcsan_test5tests8racy_inc+0x1c/0x30
|  _RNvNtNtCskctMpSfuwL9_6kernel10kcsan_test5tests28kunit_rust_wrapper_data_race+0x28c/0x850
|  kunit_try_run_case+0xdf/0x2a0
|  kunit_generic_run_threadfn_adapter+0x48/0x80
|  kthread+0x234/0x280
|  ret_from_fork+0xf8/0x280
|  ret_from_fork_asm+0x1a/0x30
|
| read-write to 0xffffffffb751ae28 of 8 bytes by task 12 on cpu 3:
|  _RNvNtNtCskctMpSfuwL9_6kernel10kcsan_test5tests8racy_inc+0x1c/0x30
|  _RNvXsb_NtCskctMpSfuwL9_6kernel9workqueueINtNtCs1G77UTwpz6o_4core3pin3PinINtNtNtB7_5alloc4kbox3BoxINtB5_11ClosureWorkNCNvNtNtB7_10kcsan_test5tests9data_races_0ENtNtB1d_9allocator7KmallocEEINtB5_15WorkItemPointerKy0_E3runB7_+0x55/0x120
|  process_scheduled_works+0x4cd/0x970
|  worker_thread+0x4de/0x6e0
|  kthread+0x234/0x280
|  ret_from_fork+0xf8/0x280
|  ret_from_fork_asm+0x1a/0x30
|
| value changed: 0x000000000000022b -> 0x00000000000005a2
| [...]

The test is intentionally kept minimal; comprehensive tests of KCSAN's
runtime behaviour are in kernel/kcsan/kcsan_test.c.

Assisted-by: Antigravity
Acked-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* Fix formatting.
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/Kconfig.test        |  11 +++
 rust/kernel/kcsan_test.rs       | 137 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 5 files changed, 152 insertions(+)
 create mode 100644 rust/kernel/kcsan_test.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index cc3cae2e378b..3fbbfbb31f5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14089,6 +14089,7 @@ F:	Documentation/dev-tools/kcsan.rst
 F:	include/linux/kcsan*.h
 F:	kernel/kcsan/
 F:	lib/Kconfig.kcsan
+F:	rust/kernel/kcsan_test.rs
 F:	scripts/Makefile.kcsan
 
 KDUMP
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 4b31aa7f432f..b06d5142d6e1 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -97,6 +97,7 @@
 #include <linux/xarray.h>
 #include <net/genetlink.h>
 #include <net/netlink.h>
+#include <trace/events/printk.h>
 #include <trace/events/rust_sample.h>
 
 /*
diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
index e6a5c7a795f0..caf98a23de76 100644
--- a/rust/kernel/Kconfig.test
+++ b/rust/kernel/Kconfig.test
@@ -83,4 +83,15 @@ config RUST_BITFIELD_KUNIT_TEST
 
 	  If unsure, say N.
 
+config RUST_KCSAN_KUNIT_TEST
+	bool "KUnit test for KCSAN instrumentation of Rust code" if !KUNIT_ALL_TESTS
+	depends on KCSAN && HAVE_KCSAN_RUSTC && TRACEPOINTS
+	default KUNIT_ALL_TESTS
+	help
+	  This option enables a KUnit test checking that KCSAN detects data
+	  races in Rust code. The test needs at least 2 CPUs to reliably
+	  observe a data race.
+
+	  If unsure, say N.
+
 endif
diff --git a/rust/kernel/kcsan_test.rs b/rust/kernel/kcsan_test.rs
new file mode 100644
index 000000000000..24810cf2970a
--- /dev/null
+++ b/rust/kernel/kcsan_test.rs
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! KUnit test checking that KCSAN detects data races in Rust code.
+
+#[macros::kunit_tests(rust_kcsan)]
+mod tests {
+    use crate::{
+        bindings,
+        error::to_result,
+        ffi::{
+            c_char,
+            c_void, //
+        },
+        prelude::*,
+        sync::{
+            atomic::{
+                Atomic,
+                Relaxed, //
+            },
+            Arc,
+            Completion, //
+        },
+        time::{
+            Delta,
+            Instant,
+            Monotonic, //
+        },
+        types::ScopeGuard,
+        workqueue, //
+    };
+    use core::{
+        cell::UnsafeCell,
+        ptr,
+        slice, //
+    };
+
+    struct Racy(UnsafeCell<u64>);
+
+    // SAFETY: Not actually safe: unsynchronized concurrent access is the point of this test.
+    unsafe impl Sync for Racy {}
+
+    static RACY: Racy = Racy(UnsafeCell::new(0));
+
+    /// Set by [`probe_console`] once KCSAN reported a data race in [`racy_inc`].
+    static OBSERVED: Atomic<bool> = Atomic::new(false);
+
+    #[inline(never)]
+    fn racy_inc() {
+        let p = RACY.0.get();
+        // SAFETY: `p` is valid; the data race is intentional. `wrapping_add()` keeps the load and
+        // store in one basic block, so that the access is instrumented as a compound access (plain
+        // writes may be assumed atomic with `CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=y`).
+        unsafe {
+            *p = (*p).wrapping_add(1);
+        }
+    }
+
+    fn contains(haystack: &[u8], needle: &[u8]) -> bool {
+        haystack.windows(needle.len()).any(|w| w == needle)
+    }
+
+    /// Probe for the `console` tracepoint.
+    ///
+    /// # Safety
+    ///
+    /// `buf` must be non-null and valid for reads of `len` bytes.
+    unsafe extern "C" fn probe_console(_data: *mut c_void, buf: *const c_char, len: usize) {
+        // SAFETY: By the safety requirements of this function.
+        let line = unsafe { slice::from_raw_parts(buf.cast::<u8>(), len) };
+
+        if contains(line, b"BUG: KCSAN: data-race in ") && contains(line, b"racy_inc") {
+            OBSERVED.store(true, Relaxed);
+        }
+    }
+
+    #[pin_data]
+    struct Ctx {
+        stop: Atomic<bool>,
+        #[pin]
+        done: Completion,
+    }
+
+    #[test]
+    fn data_race() -> Result {
+        // Do not pass on a report from a previous run of this suite.
+        OBSERVED.store(false, Relaxed);
+
+        let ctx = Arc::pin_init(
+            pin_init!(Ctx {
+                stop: Atomic::new(false),
+                done <- Completion::new(),
+            }),
+            GFP_KERNEL,
+        )?;
+
+        // SAFETY: `probe_console` has the signature expected by the `console` tracepoint. Being
+        // part of the kernel image, it and `OBSERVED` outlive the registration, so there is no
+        // need to synchronize with in-flight probes after unregistering.
+        to_result(unsafe {
+            bindings::tracepoint_probe_register(
+                &raw mut bindings::__tracepoint_console,
+                probe_console as *mut c_void,
+                ptr::null_mut(),
+            )
+        })?;
+        let _unregister = ScopeGuard::new(|| {
+            // SAFETY: The probe was registered above.
+            unsafe {
+                bindings::tracepoint_probe_unregister(
+                    &raw mut bindings::__tracepoint_console,
+                    probe_console as *mut c_void,
+                    ptr::null_mut(),
+                );
+            }
+        });
+
+        let worker = ctx.clone();
+        workqueue::system_unbound().try_spawn(GFP_KERNEL, move || {
+            while !worker.stop.load(Relaxed) {
+                racy_inc();
+            }
+            worker.done.complete_all();
+        })?;
+
+        let start = Instant::<Monotonic>::now();
+        while !OBSERVED.load(Relaxed) && start.elapsed() < Delta::from_secs(10) {
+            racy_inc();
+        }
+
+        ctx.stop.store(true, Relaxed);
+        ctx.done.wait_for_completion();
+        drop(_unregister);
+
+        assert!(OBSERVED.load(Relaxed));
+        Ok(())
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 4d5c96ddc49c..009efda9e7a4 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -94,6 +94,8 @@
 pub mod iov;
 pub mod irq;
 pub mod jump_label;
+#[cfg(CONFIG_RUST_KCSAN_KUNIT_TEST)]
+mod kcsan_test;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
 pub mod list;
-- 
2.56.0.385.gd3acb90ef8-goog