linux-kernel - RFC: Checkpatch: Introduce list of functions that need memory barriers.

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

Message-ID: <20250104211554.20205-1-manfred@colorfullife.com>
Date: Sat,  4 Jan 2025 22:15:54 +0100
From: Manfred Spraul <manfred@...orfullife.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Joe Perches <joe@...ches.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	WangYuli <wangyuli@...ontech.com>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Christian Brauner <brauner@...nel.org>,
	1vier1@....de,
	Manfred Spraul <manfred@...orfullife.com>
Subject: RFC: Checkpatch: Introduce list of functions that need memory barriers.

(depending on output of the discussion in
https://lore.kernel.org/lkml/20250102163320.GA17691@redhat.com/T/#u

It does not make sense to change it now, but I do not see a reason
to single out waitqueue_active().
The code is copy&paste, it seems to work.
There is already a recommendation for spin_is_locked()
)


2nd spinoff from the fs/pipe discussion, and ortogonal to both
the initial topic (use waitqueue_active()) and the 2nd topic
(do not wake up writers if the pipe is not writable)

Memory barriers must be paired to be effective, thus it is
mandatory to add comments that explain the pairing.
Several functions depend on the caller to take care of this.

There is already a request to add a comment for waitqueue_active(),
but there are further comparable functions:

 wq_has_sleepers(): No barrier is needed if the function
  is paired with prepare_to_wait(). With add_wait_queue(),
  a barrier is needed after the add_wait_queue() call.

- spin_is_locked(): the ACQUIRE barrier from spin_lock()
  is on the load, not on the store. Thus spin_is_locked()
  may return false even though the lock is already taken.
  Avoid to use it outside of debug code.

(and, for completeness)
- waitqueue_active(): Usually, a memory barrier before
  the call is needed, and if add_wait_queue() is used, also
  a barrier after add_wait_queue. See wait.h for details.

-
Signed-off-by: Manfred Spraul <manfred@...orfullife.com>
---
 scripts/checkpatch.pl | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..8bf5849ee108 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6685,11 +6685,17 @@ sub process {
 			     "__smp memory barriers shouldn't be used outside barrier.h and asm-generic\n" . $herecurr);
 		}
 
-# check for waitqueue_active without a comment.
-		if ($line =~ /\bwaitqueue_active\s*\(/) {
+# check for functions that are only safe with memory barriers without a comment.
+		my $need_barriers = qr{
+			waitqueue_active|
+			wq_has_sleeper|
+			spin_is_locked
+		}x;
+
+		if ($line =~ /\b(?:$need_barriers)\s*\(/) {
 			if (!ctx_has_comment($first_line, $linenr)) {
-				WARN("WAITQUEUE_ACTIVE",
-				     "waitqueue_active without comment\n" . $herecurr);
+				WARN("NEED_MEMORY_BARRIERS",
+				     "function that usually depend on manual memory barriers without comment\n" . $herecurr);
 			}
 		}
 
-- 
2.47.1