linux-kernel - [PATCH] checkpatch: check for functions with 'passed by value' structs or unions

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

Message-ID: <84972e19138f1f8eafe74c6b5f1ad874f8525706.camel@perches.com>
Date:   Thu, 26 Jul 2018 13:53:07 -0700
From:   Joe Perches <joe@...ches.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Andy Whitcroft <apw@...onical.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH] checkpatch: check for functions with 'passed by value'
 structs or unions

I was cc'd on a patch where structs were passed by value instead
of using pointers to the structs.  This can cause defects when
the called function modifies the 'passed by value' struct instead
of the calling function's struct.

There are what seems to be some false positives for a few of the
.h files in include/linux/... where the false positives are for
very small structs where the indirection via a pointer might be
slower than than the passed by value use.

Signed-off-by: Joe Perches <joe@...ches.com>
---
 scripts/checkpatch.pl | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 34e4683de7a3..7ce0ec9c0d19 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6076,8 +6076,10 @@ sub process {
 # check for function definitions
 		if ($perl_version_ok &&
 		    defined $stat &&
-		    $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) {
+		    $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*([\{;])/s) {
 			$context_function = $1;
+			my $args = $2;
+			my $term = $3;
 
 # check for multiline function definition with misplaced open brace
 			my $ok = 0;
@@ -6088,12 +6090,26 @@ sub process {
 				$herectx .=  $rl . "\n";
 				$ok = 1 if ($rl =~ /^[ \+]\{/);
 				$ok = 1 if ($rl =~ /\{/ && $n == 0);
-				last if $rl =~ /^[ \+].*\{/;
+				last if ($rl =~ /^[ \+].*[\{;]/);
 			}
-			if (!$ok) {
+			if (!$ok && $term eq '{') {
 				ERROR("OPEN_BRACE",
 				      "open brace '{' following function definitions go on the next line\n" . $herectx);
 			}
+
+# check for 'passed by value' uses of a struct or a union that might
+# be better 'passed by reference'
+
+			while ($args =~ /(?:$Storage\s+)?($Type)\s*($Ident(?:\s*\[\s*\])?)?\s*,?/g) {
+				my $type = trim($1);
+				my $ident = defined($2) ? trim($2) : "";
+				if ($type =~ /\b(?:union|struct)\b/ &&
+				    !($type =~ /(?:\*|\bconst|\])$/ ||
+				      $ident =~ /\]$/)) {
+					WARN("AGGREGATE_BY_VALUE",
+					     "Unusual 'passed by value' use of '$type'\n" . $herectx);
+				}
+			}
 		}
 
 # checks for new __setup's