linux-kernel - [PATCH] rwonce: handle KCSAN like KASAN in read_word_at_a

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250325-kcsan-rwonce-v1-1-36b3833a66ae@google.com>
Date: Tue, 25 Mar 2025 17:01:34 +0100
From: Jann Horn <jannh@...gle.com>
To: Marco Elver <elver@...gle.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
 Arnd Bergmann <arnd@...db.de>
Cc: kasan-dev@...glegroups.com, linux-arch@...r.kernel.org, 
 linux-kernel@...r.kernel.org, Jann Horn <jannh@...gle.com>
Subject: [PATCH] rwonce: handle KCSAN like KASAN in read_word_at_a_time()

read_word_at_a_time() is allowed to read out of bounds by straddling the
end of an allocation (and the caller is expected to then mask off
out-of-bounds data). This works as long as the caller guarantees that the
access won't hit a pagefault (either by ensuring that addr is aligned or by
explicitly checking where the next page boundary is).

Such out-of-bounds data could include things like KASAN redzones, adjacent
allocations that are concurrently written to, or simply an adjacent struct
field that is concurrently updated. KCSAN should ignore racy reads of OOB
data that is not actually used, just like KASAN, so (similar to the code
above) change read_word_at_a_time() to use __no_sanitize_or_inline instead
of __no_kasan_or_inline, and explicitly inform KCSAN that we're reading
the first byte.

We do have an instrument_read() helper that calls into both KASAN and
KCSAN, but I'm instead open-coding that here to avoid having to pull the
entire instrumented.h header into rwonce.h.

Also, since this read can be racy by design, we should technically do
READ_ONCE(), so add that.

Fixes: dfd402a4c4ba ("kcsan: Add Kernel Concurrency Sanitizer infrastructure")
Signed-off-by: Jann Horn <jannh@...gle.com>
---
This is a low-priority fix. I've never actually hit this issue with
upstream KCSAN.
(I only noticed it because I... err... hooked up KASAN to the KCSAN
hooks. Long story.)

I'm not sure if this should go through Arnd's tree (because it's in
rwonce.h) or Marco's (because it's a KCSAN thing).
Going through Marco's tree (after getting an Ack from Arnd) might
work a little better for me, I may or may not have more KCSAN patches
in the future.
---
 include/asm-generic/rwonce.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/rwonce.h b/include/asm-generic/rwonce.h
index 8d0a6280e982..e9f2b84d2338 100644
--- a/include/asm-generic/rwonce.h
+++ b/include/asm-generic/rwonce.h
@@ -79,11 +79,14 @@ unsigned long __read_once_word_nocheck(const void *addr)
 	(typeof(x))__read_once_word_nocheck(&(x));			\
 })

-static __no_kasan_or_inline
+static __no_sanitize_or_inline
 unsigned long read_word_at_a_time(const void *addr)
 {
+	/* open-coded instrument_read(addr, 1) */
 	kasan_check_read(addr, 1);
-	return *(unsigned long *)addr;
+	kcsan_check_read(addr, 1);
+
+	return READ_ONCE(*(unsigned long *)addr);
 }

 #endif /* __ASSEMBLY__ */

---
base-commit: 2df0c02dab829dd89360d98a8a1abaa026ef5798
change-id: 20250325-kcsan-rwonce-c990e2c1fca5

-- 
Jann Horn <jannh@...gle.com>