linux-kernel - Re: Use of '%#02x' is almost always wrong

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

Date:   Wed, 13 Oct 2021 01:17:30 -0700
From:   Joe Perches <joe@...ches.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors <kernel-janitors@...r.kernel.org>
Subject: Re: Use of '%#02x' is almost always wrong

On Wed, 2021-10-13 at 09:15 +0300, Dan Carpenter wrote:
> On Sun, Oct 10, 2021 at 09:45:30AM -0700, Joe Perches wrote:
> > Perhaps too many developers do not know that using '%#<width>x'
> > in printf output _includes_ the 0x prefix in the output width.
> > 
> 
> This is a good point.  Presumably you're going to add it to checkpatch.pl?

Yeah.  Something like the below:

> I looked at '%#04x' and you would think those would all be printing char
> types but some are printing shorts.  :/  It's harder to deal with that
> because 50% are correct from the tiny sample I looked at.

That'd be difficult as checkpatch doesn't know format argument types.
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c27d2312cfc30..97deae9dafcdc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6703,7 +6703,7 @@ sub process {
 			}
 		}
 
-# check for vsprintf extension %p<foo> misuses
+# check for vsprintf format and extension %p<foo> misuses
 		if ($perl_version_ok &&
 		    defined $stat &&
 		    $stat =~ /^\+(?![^\{]*\{\s*).*\b(\w+)\s*\(.*$String\s*,/s &&
@@ -6720,6 +6720,18 @@ sub process {
 				my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
 				$fmt =~ s/%%//g;
 
+				# check for %# (0x prefixed) width too small
+				while ($fmt =~ /(%#0?([123])([xX]))/g) {
+					my $hex = $1;
+					my $width = $2;
+					my $case = $3;
+					if (!defined($stat_real)) {
+						$stat_real = get_stat_real($linenr, $lc);
+					}
+					WARN("VSPRINTF_HEX_WIDTH",
+					     "hex specifier '$hex' - output width '$width' does not include the '0$case' prefix, width should be probably be increased\n" . "$here\n$stat_real");
+				}
+				pos($fmt) = 0;
 				while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) {
 					$specifier = $1;
 					$extension = $2;