[PATCH] checkpatch: warn for use of %px

["Tobin C. Harding" <me@tobin.cc>]

Usage of the new %px specifier potentially leaks sensitive
inforamtion. Printing kernel addresses exposes the kernel layout in
memory, this is potentially exploitable. We have tools in the kernel to
help us do the right thing. We can have checkpatch warn developers of
potential dangers of using %px.

Have checkpatch emit a warning for usage of specifier %px.

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Tobin C. Harding <me@tobin.cc>
Co-Developed-by: Joe Perches <joe@perches.com>

---

Joe,

Are you happy with this tagging? Needs your signed-off-by still.


Andrew,

Is it okay to add your Suggested-by tag here?

I'm not entirely sure when one is supposed to add someones signed-off-by
tag since the docs state that it should not be added without
permission. I am also unsure where/when is the best time to request this
permission.

thanks,
Tobin.


 scripts/checkpatch.pl | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 040aa79e1d9d..c63466f86b18 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1612,6 +1612,17 @@ sub raw_line {
 	return $line;
 }
 
+sub stat_real {
+	my ($linenr, $lc) = @_;
+
+	my $stat_real = raw_line($linenr, 0);
+	for (my $count = $linenr + 1; $count <= $lc; $count++) {
+		$stat_real = $stat_real . "\n" . raw_line($count, 0);
+	}
+
+	return $stat_real;
+}
+
 sub cat_vet {
 	my ($vet) = @_;
 	my ($res, $coded);
@@ -5747,24 +5758,35 @@ sub process {
 		    defined $stat &&
 		    $stat =~ /^\+(?![^\{]*\{\s*).*\b(\w+)\s*\(.*$String\s*,/s &&
 		    $1 !~ /^_*volatile_*$/) {
-			my $bad_extension = "";
+			my ($specifier, $extension, $stat_real);
+			my $bad_specifier = "";
 			my $lc = $stat =~ tr@\n@@;
 			$lc = $lc + $linenr;
 		        for (my $count = $linenr; $count <= $lc; $count++) {
 				my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
 				$fmt =~ s/%%//g;
-				if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNOx]).)/) {
-					$bad_extension = $1;
-					last;
+
+				while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) {
+					$specifier = $1;
+					$extension = $2;
+					if ($extension !~ /[FfSsBKRraEhMmIiUDdgVCbGNOx]/) {
+						$bad_specifier = $specifier;
+						last;
+					}
+					if ($extension eq "x" && !defined($stat_real)) {
+						if (!defined($stat_real)) {
+							$stat_real = stat_real($linenr, $lc);
+						}
+						WARN("VSPRINTF_SPECIFIER_PX",
+						     "Using vsprintf specifier '\%px' potentially exposes the kernel layout in memory, if you don't _realy_ need the address please consider using '\%p'.\n" . "$here\n$stat_real\n");
+					}
 				}
+
 			}
-			if ($bad_extension ne "") {
-				my $stat_real = raw_line($linenr, 0);
-				for (my $count = $linenr + 1; $count <= $lc; $count++) {
-					$stat_real = $stat_real . "\n" . raw_line($count, 0);
-				}
+			if ($bad_specifier ne "") {
+				$stat_real = stat_real($linenr, $lc);
 				WARN("VSPRINTF_POINTER_EXTENSION",
-				     "Invalid vsprintf pointer extension '$bad_extension'\n" . "$here\n$stat_real\n");
+				     "Invalid vsprintf pointer extension '$bad_specifier'\n" . "$here\n$stat_real\n");
 			}
 		}
 
-- 
2.7.4