scripts: leaking_addresses.pl: add support for 32-bit kernel addresses

The current leaking_addresses.pl script only supports showing "leaked" 64-bit kernel virtual addresses. This patch adds support for showing "leaked" 32-bit kernel virtual addresses.

The way it currently works- once it detects we're running on an i'x'86 platform (where x=3|4|5|6), it takes this arch into account for checking: the essential rationale:
virt-addr >= PAGE_OFFSET => it's a kernel virtual address.

Note-
1. It's a work in progress; some pending TODOs:

- support for ARM-32

- programatically query and set the PAGE_OFFSET based on arch (it's currently hard-coded)

2. Minor edit:
the '--raw', '--suppress-dmesg', '--squash-by-path' and '--squash-by-filename' option switches are only meaningful
when the '----input-raw=' option is used. So, indent the 'Help' screen lines to reflect the fact.

Feedback welcome..

Signed-off-by Kaiwan N Billimoria <[email protected]>

---

scripts/leaking_addresses.pl +0 -0

index bc5788000018..e139de445ad1 100755

scripts/leaking_addresses.pl +1 -1

 use warnings;
 use strict;
 use POSIX;

@@ -35,7 +38,7 @@ my $TIMEOUT = 10;
 # Script can only grep for kernel addresses on the following architectures. If
 # your architecture is not listed here and has a grep'able kernel address please
 # consider submitting a patch.
-my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
+my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
 
 # Command line options.
 my $help = 0;

scripts/leaking_addresses.pl +0 -0

 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',
 			    '/proc/kcore',

@@ -97,10 +103,10 @@ Options:
 
 	-o, --output-raw=<file>  Save results for future processing.
 	-i, --input-raw=<file>   Read results from file instead of scanning.

scripts/leaking_addresses.pl +1 -1

@@ -177,7 +183,7 @@ sub dprint
 
 sub is_supported_architecture
 {
-	return (is_x86_64() or is_ppc64());
+	return (is_x86_64() or is_ppc64() or is_ix86_32());
 }
 
 sub is_x86_64

You don't typically need [xxx v1] for version 1, the v1 is implicit.

Please use the git brief description prefix that is already in use i.e

leaking_addresses: add support for 32-bit kernel addresses

Quoted 1 line

- support for ARM-32

Sure, we can do this later.

Quoted 3 lines

- programatically query and set the PAGE_OFFSET based on arch (it's currently hard-coded)

Let's do this straight away, it will be much nicer.

Quoted 5 lines

2. Minor edit:
the '--raw', '--suppress-dmesg', '--squash-by-path' and '--squash-by-filename' option switches are only meaningful
when the '----input-raw=' option is used. So, indent the 'Help' screen lines to reflect the fact.

This is a different change to the architecture stuff so should be in a separate patch. You could do both as a series if you like. Off the top of my head I have never seen options output like this, but if you have, I'm willing to accept your view. You are correct that the options mentioned are only use in conjuncture with '--input-raw' so some way of showing this would be nice.

Quoted 2 lines

+my $bit_size = 64; # Check 64-bit kernel addresses by default

This global is unnecessary. You already have is_ix86_32() so you can just use that.

From your later comments, I think you see that using this global is necessary.

Please use kernel coding style

                $bit_size = 32;

Bonus points, you uncovered a bug in the current script if (is_x86_64) was missing the parenthesis!

scripts/leaking_addresses.pl sub is_false_positive · line 245

Quoted 4 lines

scripts/leaking_addresses.pl +3 -0

+             if ($match =~ '\b(0x)?(f|F){8}\b') {
+                     return 1;
+             }

So, may_leak_address() and is_false_positive() are tightly coupled and not really that nice. Once we add 32 bit support it gets worse. Going forwards, we can either add your 32 bit work then refactor both functions or you can refactor them as you add the 32 bit stuff. I'm open to either.

Some things to note

- The mask stuff (all 1's) should have an all 0's regex also.

- The mask stuff should probably be closer to the mask stuff for 64 bit. It's not immediately apparent a clean way to do this though.
- It's not immediately apparent if an address less that PAGE_OFFSET is a false positive or should be caught in leaks_address().

- Do we need 32 bit equivalents for

        if ($line =~ '\bKEY=[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b' or
            $line =~ '\b[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b') {

Your patch did not apply, the problem looks to be in the code section above. You can see that there is no removed line. For next spin please check your patch applies on top of the 'leaks' branch (which now includes the fix for the bug you found).

I have one more comment that should have been at the top but I did not want to confuse things. Typically, the git brief description should be limited to 50 characters. If you do decide to split this patch into two and use the prefix suggested you may like to change the git brief description but don't feel you have to. If you do decide to do this, your next patch set will be a version 1 again. I may be wrong but I never increment a patch version if the subject line changes (excluding contents of [] ).

thanks,
Tobin.