linux-kernel - Re: [PATCH] atomic: Specify natural alignment for atomic

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250901093600.GF4067720@noisy.programming.kicks-ass.net>
Date: Mon, 1 Sep 2025 11:36:00 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Finn Thain <fthain@...ux-m68k.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Lance Yang <lance.yang@...ux.dev>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Eero Tamminen <oak@...sinkinet.fi>, Will Deacon <will@...nel.org>,
	stable@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] atomic: Specify natural alignment for atomic_t

On Thu, Aug 28, 2025 at 07:53:52PM +1000, Finn Thain wrote:

> > Anyway, I'm not opposed to adding an explicit alignment to atomic_t.
> > Isn't s32 or __s32 already having this?
> > 
> 
> For Linux/m68k, __alignof__(__s32) == 2 and __alignof__(s32) == 2.

Hmm, somehow I thought one of those enforced natural alignment. Oh well.

> > But I think it might make sense to have a DEBUG alignment check right
> > along with adding that alignment, just to make sure things are indeed /
> > stay aligned.
> > 
> 
> A run-time assertion seems surperfluous as long as other architectures 
> already trap for misaligned locks. 

Right, but those architectures have natural alignment. m68k is 'special'
in that it doesn't have this.

> For m68k, perhaps we could have a compile-time check:

I don't think build-time is sufficient. There is various code that casts
to atomic types and other funny stuff like that.

If you want to ensure 'atomics' are always naturally aligned, the only
sound way is to have a runtime test/trap.

Something like the completely untested below should do I suppose.

---
diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
index 711a1f0d1a73..e39cdfe5a59e 100644
--- a/include/linux/instrumented.h
+++ b/include/linux/instrumented.h
@@ -67,6 +67,7 @@ static __always_inline void instrument_atomic_read(const volatile void *v, size_
 {
 	kasan_check_read(v, size);
 	kcsan_check_atomic_read(v, size);
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & 3));
 }
 
 /**
@@ -81,6 +82,7 @@ static __always_inline void instrument_atomic_write(const volatile void *v, size
 {
 	kasan_check_write(v, size);
 	kcsan_check_atomic_write(v, size);
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & 3));
 }
 
 /**
@@ -95,6 +97,7 @@ static __always_inline void instrument_atomic_read_write(const volatile void *v,
 {
 	kasan_check_write(v, size);
 	kcsan_check_atomic_read_write(v, size);
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & 3));
 }
 
 /**
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dc0e0c6ed075..1c7e30cdfe04 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1363,6 +1363,16 @@ config DEBUG_PREEMPT
 	  depending on workload as it triggers debugging routines for each
 	  this_cpu operation. It should only be used for debugging purposes.
 
+config DEBUG_ATOMIC
+	bool "Debug atomic variables"
+	depends on DEBUG_KERNEL
+	help
+	  If you say Y here then the kernel will add a runtime alignment check
+	  to atomic accesses. Useful for architectures that do not have trap on
+	  mis-aligned access.
+
+	  This option has potentially significant overhead.
+
 menu "Lock Debugging (spinlocks, mutexes, etc...)"
 
 config LOCK_DEBUGGING_SUPPORT