linux-kernel - [PATCH] checkpatch: allow tags between co-developed-by and their sign-off

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: <20231020132156.37882-1-przemyslaw.kitszel@intel.com>
Date:   Fri, 20 Oct 2023 15:21:56 +0200
From:   Przemek Kitszel <przemyslaw.kitszel@...el.com>
To:     Andy Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        linux-kernel@...r.kernel.org
Cc:     Przemek Kitszel <przemyslaw.kitszel@...el.com>,
        Jacob Keller <jacob.e.keller@...el.com>
Subject: [PATCH] checkpatch: allow tags between co-developed-by and their sign-off

Allow additional tags between Co-developed-by: and Signed-off-by:.
Bump severity of missing SoB to ERROR.

Additional tags between Co-developed-by and corresponding Signed-off-by
could include Reviewed-by tags collected by Submitter, which is also
a Co-developer, but should sign-off at the very end of tags provided by
themself.

Missing SoB is promoted to error while that piece of code is touched.

Two sets of perl %hashes introduced to keep both (int) line numbers and
(string) messages handy for warning reporting, while keeping it correct
across 100+ line long commit messages.

Mateusz Polchlopek <mateusz.polchlopek@...el.com> has reported this to me.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
---
 scripts/checkpatch.pl | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7d16f863edf1..0400bf092bfa 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2682,6 +2682,10 @@ sub process {
 	my $suppress_statement = 0;
 
 	my %signatures = ();
+	my %signoffs = ();
+	my %signoffs_msg = ();
+	my %codevs = ();
+	my %codevs_msg = ();
 
 	# Pre-scan the patch sanitizing the lines.
 	# Pre-scan the patch looking for any __setup documentation.
@@ -2967,11 +2971,13 @@ sub process {
 		if ($line =~ /^\s*signed-off-by:\s*(.*)/i) {
 			$signoff++;
 			$in_commit_log = 0;
+			my $ctx = $1;
+			$signoffs{$ctx} = $linenr;
+			$signoffs_msg{$ctx} = $herecurr;
 			if ($author ne ''  && $authorsignoff != 1) {
-				if (same_email_addresses($1, $author)) {
+				if (same_email_addresses($ctx, $author)) {
 					$authorsignoff = 1;
 				} else {
-					my $ctx = $1;
 					my ($email_name, $email_comment, $email_address, $comment1) = parse_email($ctx);
 					my ($author_name, $author_comment, $author_address, $comment2) = parse_email($author);
 
@@ -3158,22 +3164,15 @@ sub process {
 				$signatures{$sig_nospace} = 1;
 			}
 
-# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email
+# Collect Co-developed-by: to check if each is backed up by Signed-off-by: with
+# the same name and email. Checks are made after main loop.
 			if ($sign_off =~ /^co-developed-by:$/i) {
 				if ($email eq $author) {
 					WARN("BAD_SIGN_OFF",
 					      "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . $herecurr);
 				}
-				if (!defined $lines[$linenr]) {
-					WARN("BAD_SIGN_OFF",
-					     "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr);
-				} elsif ($rawlines[$linenr] !~ /^signed-off-by:\s*(.*)/i) {
-					WARN("BAD_SIGN_OFF",
-					     "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr . $rawlines[$linenr] . "\n");
-				} elsif ($1 ne $email) {
-					WARN("BAD_SIGN_OFF",
-					     "Co-developed-by and Signed-off-by: name/email do not match\n" . $herecurr . $rawlines[$linenr] . "\n");
-				}
+				$codevs{$email} = $linenr;
+				$codevs_msg{$email} = $herecurr;
 			}
 
 # check if Reported-by: is followed by a Closes: tag
@@ -7712,6 +7711,17 @@ sub process {
 				     "From:/Signed-off-by: email subaddress mismatch: $sob_msg\n");
 			}
 		}
+		# check if each Co-developed-by tag is backed up by Sign-off,
+		# warn if Co-developed-by tag was put after a Signed-off-by tag
+		foreach my $codev (keys %codevs) {
+			if (!$signoffs{$codev}) {
+				ERROR("BAD_SIGN_OFF",
+				      "Co-developed-by: must be followed by Signed-off-by:\n" . $codevs_msg{$codev});
+			} elsif ($signoffs{$codev} <= $codevs{$codev}) {
+				WARN("BAD_SIGN_OFF",
+				     "Co-developed-by: must be followed by Signed-off-by:, but was placed after it\n" . $signoffs_msg{$codev} . $codevs_msg{$codev});
+			}
+		}
 	}
 
 	print report_dump();
-- 
2.38.1