linux-kernel - [PATCH v2] checkpatch: allow an assignment in if condition for ACQUIRE

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250815010645.2980846-1-alison.schofield@intel.com>
Date: Thu, 14 Aug 2025 18:06:43 -0700
From: alison.schofield@...el.com
To: Joe Perches <joe@...ches.com>,
	Andy Whitcroft <apw@...onical.com>,
	Dwaipayan Ray <dwaipayanray1@...il.com>,
	Lukas Bulwahn <lukas.bulwahn@...il.com>
Cc: Alison Schofield <alison.schofield@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Dan Williams <dan.j.williams@...el.com>,
	linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH v2] checkpatch: allow an assignment in if condition for ACQUIRE_ERR()

From: Alison Schofield <alison.schofield@...el.com>

New helpers, ACQUIRE() and ACQUIRE_ERR(), were recently introduced
and employed here [1] to clean up conditional locking paths.

That led to checkpatch ERRORS:
        ERROR: do not use assignment in if condition

on usages like this:
        ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
        if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
                return rc;

That compact format was a deliberate choice by the authors. By making
this a checkpatch exception, existing ERRORs are quieted, and future
users of the macro will not be dissuaded by checkpatch from using the
preferred compact format.

[1] Commit d03fcf50ba56 ("cxl: Convert to ACQUIRE() for conditional rwsem locking")

Signed-off-by: Alison Schofield <alison.schofield@...el.com>
---

Changes in v2:
- Remove next that skipped other rules (Joe)
- Replace \w+ with $Lval for more precise pattern match (Joe)
- Only allow lines where all assignments are to ACQUIRE_ERR
- Tested many more allow and trigger conditions.
  Will share in reply to this post.
- Update commit message

 scripts/checkpatch.pl | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e722dd6fa8ef..30435967d8c4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5696,7 +5696,18 @@ sub process {
 			my ($s, $c) = ($stat, $cond);
 			my $fixed_assign_in_if = 0;
 
-			if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) {
+			if ($c =~ /\bif\s*\((.*[^<>!=]=[^=].*)\)/s) {
+			    my $condition = $1;
+			    my $allow_assignment = 1;
+
+			    # Allow single ACQUIRE_ERR assignment, reject everything else
+			    while ($condition =~ /\b($Lval)\s*=\s*([^,)&|]+)/g) {
+				 if ($2 !~ /^\s*ACQUIRE_ERR\s*\(/) {
+					$allow_assignment = 0;
+					last;
+				}
+			    }
+			    if (!$allow_assignment) {
 				if (ERROR("ASSIGN_IN_IF",
 					  "do not use assignment in if condition\n" . $herecurr) &&
 				    $fix && $perl_version_ok) {
@@ -5721,6 +5732,7 @@ sub process {
 						fix_insert_line($fixlinenr + 1, $newline);
 						$fixed_assign_in_if = 1;
 					}
+				    }
 				}
 			}
 

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.37.3