leaking_addresses: add generic 32-bit support

The script now attempts to detect the architecture it's running upon; as of now, we explicitly support x86_64, PPC64, ARM64, MIPS64 and x86_32.

If it's one of them, we proceed "normally". If we fail to detect the arch, we fallback to 64-bit scanning, _unless_ the user has passed either of these option switches: "--opt-32bit" and/or "--page-offset-32bit=<val>".

If so, we switch to scanning for leaked addresses based on the value of PAGE_OFFSET (via an auto-detected or fallback mechanism).

Currently, code (or "rules") to detect special cases for x86_64, PPC64 and Aarch64 (in the get_address_re sub) is present. Also, we now have also builtin "stubs", for lack of a better term, where additional rules for other 64-bit arch's can be plugged into the code, in future, as applicable.

This patch adds support for ix86 and generic 32 bit architectures.
- Add command line option for generic 32-bit checking.
- Add command line option for page offset.
- Add command line option for kernel configuration file.
- Parse kernel config file for page offset (CONFIG_PAGE_OFFSET).
- Use page offset when checking for kernel virtual addresses.

The script has been lightly tested on the following systems:
x86_64
x86_32 (on a i686 running Debian 7 to be precise)
ARM32 (on a Yocto-based qemuarm32 ARM Versatile platform (ARM926EJ-S cpu))

In the first two cases, one just has to run the script - no parameters required. In the ARM-32 case, it will, by design, fail to detect the architecture; (re)running it with the '--32-bit' and/or the '--page-offset-32bit=<hexval>' option switch(es) has the desired effect: it now detects 32-bit leaking kernel addresses, if any.

Request more testing on the above and other platforms.

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

---
 scripts/leaking_addresses.pl | 190 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 156 insertions(+), 34 deletions(-)

scripts/leaking_addresses.pl +156 -34

index a29e13e577a7..b0807b3a3c7c 100755
@@ -1,10 +1,10 @@
 #!/usr/bin/env perl
 #
 # (c) 2017 Tobin C. Harding <[email protected]>
-
+# (c) 2017 Kaiwan N Billimoria <[email protected]>
 # Licensed under the terms of the GNU GPL License version 2
 #
-# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
+# leaking_addresses.pl: Scan kernel for potential leaking addresses.
 #  - Scans dmesg output.
 #  - Walks directory tree and parses each file (for each directory in @DIRS).
 #

@@ -32,11 +32,6 @@ my @DIRS = ('/proc', '/sys');
 # Timer for parsing each file, in seconds.
 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');
-
 # Command line options.
 my $help = 0;
 my $debug = 0;

@@ -48,7 +43,9 @@ my $suppress_dmesg = 0;		# Don't show dmesg in output.
 my $squash_by_path = 0;		# Summary report grouped by absolute path.
 my $squash_by_filename = 0;	# Summary report grouped by filename.
 
-my $kernel_config_file = "";	# Kernel configuration file.
+my $opt_32_bit = 0;            # Detect (only) 32-bit kernel leaking addresses.
+my $page_offset_32bit = 0;     # 32-bit: value of CONFIG_PAGE_OFFSET.
+my $kernel_config_file = "";   # Kernel configuration file.
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',

@@ -104,10 +101,12 @@ Options:
 	      --squash-by-path		Show one result per unique path.
 	      --squash-by-filename	Show one result per unique filename.
 	--kernel-config-file=<file>     Kernel configuration file (e.g /boot/config)
+	--opt-32bit			Detect (only) 32-bit kernel leaking addresses.
+	--page-offset-32bit=<hex>	PAGE_OFFSET value (for 32-bit kernels).
 	-d, --debug			Display debugging output.
-	-h, --help, --versionq		Display this help and exit.
+	-h, --help, --version		Display this help and exit.
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+Scans the running kernel for potential leaking addresses.
 
 EOM
 	exit($exitcode);

@@ -123,7 +122,9 @@ GetOptions(
 	'squash-by-path'        => \$squash_by_path,
 	'squash-by-filename'    => \$squash_by_filename,
 	'raw'                   => \$raw,
-	'kernel-config-file=s'	=> \$kernel_config_file,
+	'opt-32bit'             => \$opt_32_bit,
+	'page-offset-32bit=o'   => \$page_offset_32bit,
+	'kernel-config-file=s'  => \$kernel_config_file,
 ) or help(1);
 
 help(0) if ($help);

@@ -139,16 +140,15 @@ if (!$input_raw and ($squash_by_path or $squash_by_filename)) {
 	exit(128);
 }
 
-if (!is_supported_architecture()) {
-	printf "\nScript does not support your architecture, sorry.\n";
-	printf "\nCurrently we support: \n\n";
-	foreach(@SUPPORTED_ARCHITECTURES) {
-		printf "\t%s\n", $_;
-	}
+show_detected_architecture() if $debug;
 
-	my $archname = $Config{archname};
-	printf "\n\$ perl -MConfig -e \'print \"\$Config{archname}\\n\"\'\n";
-	printf "%s\n", $archname;
+if (!is_known_architecture()) {
+	printf STDERR "\nFATAL: Script does not recognize your architecture\n";
+
+	my $arch = `uname -m`;
+	chomp $arch;
+	printf "\n\$ uname -m\n";
+	printf "%s\n", $arch;
 
 	exit(129);
 }

@@ -168,21 +168,45 @@ sub dprint
 	printf(STDERR @_) if $debug;
 }
 
-sub is_supported_architecture
+sub is_known_architecture
 {
-	return (is_x86_64() or is_ppc64());
+	return (is_64bit() or is_32bit());
 }
 
-sub is_x86_64
+sub is_32bit
 {
-	my $archname = $Config{archname};
+	# 32-bit actual case
+	if (is_ix86_32()) {
+		return 1;
+	}
 
-	if ($archname =~ m/x86_64/) {
+	# 32-bit "forced" case (for any arch other than IA-32)
+	if ($opt_32_bit or $page_offset_32bit) {
 		return 1;
 	}
 	return 0;
 }
 
+sub is_64bit
+{
+	return (is_x86_64() or is_ppc64() or is_arm64() or is_mips64());
+}
+
+sub is_x86_64
+{
+	is_arch("x86_64");
+}
+
+sub is_arm64
+{
+	is_arch("aarch64");
+}
+
+sub is_mips64
+{
+	is_arch("mips64");
+}
+
 sub is_ppc64
 {
 	my $archname = $Config{archname};

@@ -193,6 +217,47 @@ sub is_ppc64
 	return 0;
 }
 
+sub is_ix86_32
+{
+	my $arch = `uname -m`;
+
+	chomp $arch;
+	if ($arch =~ m/i[3456]86/) {
+		return 1;
+	}
+	return 0;
+}
+
+sub is_arch
+{
+	my ($desc) = @_;
+	my $arch = `uname -m`;
+
+	chomp $arch;
+	if ($arch eq $desc) {
+		return 1;
+	}
+	return 0;
+}
+
+sub show_detected_architecture
+{
+	printf "Detected architecture: ";
+	if (is_32bit()) {
+		printf "32 bit\n";
+	} elsif (is_x86_64()) {
+		printf "x86_64\n";
+	} elsif (is_ppc64()) {
+		printf "PPC64\n";
+	} elsif (is_arm64()) {
+		printf "ARM64\n";
+	} elsif (is_mips64()) {
+		printf "MIPS64\n";
+	} else {
+		printf "failed to detect architecture\n"
+	}
+}
+
 # gets config option value from kernel config file
 sub get_kernel_config_option
 {

@@ -212,7 +277,6 @@ sub get_kernel_config_option
 		} else {
 			@config_files = ($tmp_file);
 		}
-
 	} else {
 		my $file = '/boot/config-' . `uname -r`;
 		chomp $file;

@@ -220,7 +284,6 @@ sub get_kernel_config_option
 	}
 
 	foreach my $file (@config_files) {
-		dprint("parsing config file: %s\n", $file);
 		$value = option_from_file($option, $file);
 		if ($value ne "") {
 			last;

@@ -258,6 +321,14 @@ sub is_false_positive
 {
 	my ($match) = @_;
 
+	# 32 bit architectures, actual or forced
+
+	if (is_32bit()) {
+		return is_false_positive_32bit($match);
+	}
+
+	# 64 bit architectures
+
 	if ($match =~ '\b(0x)?(f|F){16}\b' or
 	    $match =~ '\b(0x)?0{16}\b') {
 		return 1;

@@ -281,6 +352,58 @@ sub is_in_vsyscall_memory_region
 	return ($hex >= $region_min and $hex <= $region_max);
 }
 
+sub is_false_positive_32bit
+{
+	my ($match) = @_;
+	state $page_offset = get_page_offset(); # only gets called once
+
+	if ($match =~ '\b(0x)?(f|F){8}\b') {
+		return 1;
+	}
+
+	my $addr32 = hex($match);
+	if ($addr32 < $page_offset) {
+		return 1;
+	}
+
+	return 0;
+}
+
+sub get_page_offset
+{
+	my $page_offset;
+	my $default_offset = hex("0xc0000000");
+	my @config_files;
+
+	# Allow --page-offset-32bit to override.
+	if ($page_offset_32bit != 0) {
+		return $page_offset_32bit;
+	}
+
+	$page_offset = get_kernel_config_option('CONFIG_PAGE_OFFSET');
+	return $default_offset;
+}
+
+sub parse_kernel_config_file
+{
+	my ($file) = @_;
+	my $config = 'CONFIG_PAGE_OFFSET';
+	my $str = "";
+	my $val = "";
+
+	open(my $fh, "<", $file) or return "";
+	while (my $line = <$fh> ) {
+		if ($line =~ /^$config/) {
+			($str, $val) = split /=/, $line;
+			chomp($val);
+			last;
+		}
+	}
+
+	close $fh;
+	return $val;
+}
+
 # True if argument potentially contains a kernel address.
 sub may_leak_address
 {

@@ -300,8 +423,6 @@ sub may_leak_address
 	}
 
 	$address_re = get_address_re();
-	dprint("Kernel address regular expression: %s\n", $address_re);
-
 	while (/($address_re)/g) {
 		if (!is_false_positive($1)) {
 			return 1;

@@ -313,16 +434,17 @@ sub may_leak_address
 
 sub get_address_re
 {
-	my $re;
+	my $re = "";
 
 	if (is_x86_64()) {
 		$re = get_x86_64_re();
 	} elsif (is_ppc64()) {
 		$re = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
-	}
-
-	if ($re eq "") {
-		print STDERR "$0: failed to build kernel address regular expression\n";
+	###
+	# Any special cases for other arch's go below this line
+	###
+	} else {  # nothing? then we assume it's a generic 32-bit
+		$re = '\b(0x)?[[:xdigit:]]{8}\b';
 	}
 
 	return $re;

The script now attempts to detect the architecture it's running upon; as of now, we explicitly support x86_64, PPC64, ARM64, MIPS64 and x86_32.

If it's one of them, we proceed "normally". If we fail to detect the arch, we fallback to 64-bit scanning, _unless_ the user has passed either of these option switches: "--opt-32bit" and/or "--page-offset-32bit=<val>".

If so, we switch to scanning for leaked addresses based on the value of PAGE_OFFSET (via an auto-detected or fallback mechanism).

Currently, code (or "rules") to detect special cases for x86_64, PPC64 and Aarch64 (in the get_address_re sub) is present. Also, we now have also builtin "stubs", for lack of a better term, where additional rules for other 64-bit arch's can be plugged into the code, in future, as applicable.

This patch adds support for ix86 and generic 32 bit architectures.
- Add command line option for generic 32-bit checking.
- Add command line option for page offset.
- Add command line option for kernel configuration file.
- Parse kernel config file for page offset (CONFIG_PAGE_OFFSET).
- Use page offset when checking for kernel virtual addresses.

The script has been lightly tested on the following systems:
x86_64
x86_32 (on a i686 running Debian 7 to be precise)
ARM32 (on a Yocto-based qemuarm32 ARM Versatile platform (ARM926EJ-S cpu))

In the first two cases, one just has to run the script - no parameters required. In the ARM-32 case, it will, by design, fail to detect the architecture; (re)running it with the '--32-bit' and/or the '--page-offset-32bit=<hexval>'

option switch(es) has the desired effect: it now detects 32-bit leaking kernel addresses, if any.

Request more testing on the above and other platforms.

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

---
 scripts/leaking_addresses.pl | 190 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 156 insertions(+), 34 deletions(-)

scripts/leaking_addresses.pl +0 -0

index a29e13e577a7..b0807b3a3c7c 100755

scripts/leaking_addresses.pl +2 -6

+# (c) 2017 Kaiwan N Billimoria <[email protected]>
 # Licensed under the terms of the GNU GPL License version 2
 #
-# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
+# leaking_addresses.pl: Scan kernel for potential leaking addresses.
 #  - Scans dmesg output.
 #  - Walks directory tree and parses each file (for each directory in @DIRS).
 #

@@ -32,11 +32,6 @@ my @DIRS = ('/proc', '/sys');
 # Timer for parsing each file, in seconds.
 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');
-
 # Command line options.
 my $help = 0;
 my $debug = 0;

scripts/leaking_addresses.pl +7 -12

+	--page-offset-32bit=<hex>	PAGE_OFFSET value (for 32-bit kernels).
 	-d, --debug			Display debugging output.
-	-h, --help, --versionq		Display this help and exit.
+	-h, --help, --version		Display this help and exit.
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+Scans the running kernel for potential leaking addresses.
 
 EOM
 	exit($exitcode);

@@ -123,7 +122,9 @@ GetOptions(
 	'squash-by-path'        => \$squash_by_path,
 	'squash-by-filename'    => \$squash_by_filename,
 	'raw'                   => \$raw,
-	'kernel-config-file=s'	=> \$kernel_config_file,
+	'opt-32bit'             => \$opt_32_bit,
+	'page-offset-32bit=o'   => \$page_offset_32bit,
+	'kernel-config-file=s'  => \$kernel_config_file,
 ) or help(1);
 
 help(0) if ($help);

@@ -139,16 +140,15 @@ if (!$input_raw and ($squash_by_path or $squash_by_filename)) {
 	exit(128);
 }
 
-if (!is_supported_architecture()) {
-	printf "\nScript does not support your architecture, sorry.\n";
-	printf "\nCurrently we support: \n\n";
-	foreach(@SUPPORTED_ARCHITECTURES) {
-		printf "\t%s\n", $_;
-	}
+show_detected_architecture() if $debug;
 
-	my $archname = $Config{archname};
-	printf "\n\$ perl -MConfig -e \'print \"\$Config{archname}\\n\"\'\n";
-	printf "%s\n", $archname;

scripts/leaking_addresses.pl +4 -3

 {
-	return (is_x86_64() or is_ppc64());
+	return (is_64bit() or is_32bit());
 }
 
-sub is_x86_64
+sub is_32bit
 {
-	my $archname = $Config{archname};
+	# 32-bit actual case
+	if (is_ix86_32()) {

scripts/leaking_addresses.pl +10 -0

+sub is_64bit
+{
+	return (is_x86_64() or is_ppc64() or is_arm64() or is_mips64());
+}
+
+sub is_x86_64
+{
+	is_arch("x86_64");
+}
+

scripts/leaking_addresses.pl +18 -0

 sub is_ppc64
 {
 	my $archname = $Config{archname};

@@ -193,6 +217,47 @@ sub is_ppc64
 	return 0;
 }
 
+sub is_ix86_32
+{
+	my $arch = `uname -m`;
+
+	chomp $arch;
+	if ($arch =~ m/i[3456]86/) {
+		return 1;
+	}
+	return 0;
+}
+
+sub is_arch
+{
+	my ($desc) = @_;
+	my $arch = `uname -m`;
+
+	chomp $arch;
+	if ($arch eq $desc) {

scripts/leaking_addresses.pl +13 -0

+	} elsif (is_x86_64()) {
+		printf "x86_64\n";
+	} elsif (is_ppc64()) {
+		printf "PPC64\n";
+	} elsif (is_arm64()) {
+		printf "ARM64\n";
+	} elsif (is_mips64()) {
+		printf "MIPS64\n";
+	} else {
+		printf "failed to detect architecture\n"
+	}
+}
+
 # gets config option value from kernel config file
 sub get_kernel_config_option
 {

scripts/leaking_addresses.pl +6 -0

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

@@ -281,6 +352,58 @@ sub is_in_vsyscall_memory_region
 	return ($hex >= $region_min and $hex <= $region_max);
 }
 
+sub is_false_positive_32bit
+{
+	my ($match) = @_;
+	state $page_offset = get_page_offset(); # only gets called once
+
+	if ($match =~ '\b(0x)?(f|F){8}\b') {

scripts/leaking_addresses.pl +4 -0

+sub get_page_offset
+{
+	my $page_offset;
+	my $default_offset = hex("0xc0000000");

scripts/leaking_addresses.pl +11 -0

+
+sub parse_kernel_config_file
+{
+	my ($file) = @_;
+	my $config = 'CONFIG_PAGE_OFFSET';
+	my $str = "";
+	my $val = "";
+
+	open(my $fh, "<", $file) or return "";
+	while (my $line = <$fh> ) {
+		if ($line =~ /^$config/) {

scripts/leaking_addresses.pl +1 -3

 	while (/($address_re)/g) {
 		if (!is_false_positive($1)) {
 			return 1;

@@ -313,16 +434,17 @@ sub may_leak_address
 
 sub get_address_re
 {
-	my $re;
+	my $re = "";
 
 	if (is_x86_64()) {
 		$re = get_x86_64_re();
 	} elsif (is_ppc64()) {
 		$re = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
-	}
-

Signature · 1 line

2.14.3

Hi Kaiwan,

Thanks for the patch

Quoted 1 line · On Thu, Jan 04, 2018 at 05:51:25PM +0530, [email protected] wrote:

The script now attempts to detect the architecture it's running upon; as

of now,

Quoted 1 line

we explicitly support x86_64, PPC64, ARM64, MIPS64 and x86_32.

This is incorrect. We do not currently support ARM64, MIPS64 and x86_32.

Quoted 1 line

If it's one of them, we proceed "normally". If we fail to detect the

arch,

Quoted 1 line

we fallback to 64-bit scanning, _unless_ the user has passed either of

these

Quoted 6 lines

option switches: "--opt-32bit" and/or "--page-offset-32bit=<val>".

If so, we switch to scanning for leaked addresses based on the value of PAGE_OFFSET (via an auto-detected or fallback mechanism).

Currently, code (or "rules") to detect special cases for x86_64, PPC64

and Aarch64

Quoted 1 line

(in the get_address_re sub) is present. Also, we now have also builtin

"stubs",

Quoted 1 line

for lack of a better term, where additional rules for other 64-bit

arch's can be

Quoted 8 lines

plugged into the code, in future, as applicable.

This patch adds support for ix86 and generic 32 bit architectures.
- Add command line option for generic 32-bit checking.
- Add command line option for page offset.
- Add command line option for kernel configuration file.
- Parse kernel config file for page offset (CONFIG_PAGE_OFFSET).
- Use page offset when checking for kernel virtual addresses.

This is all pretty confusing. It is hard to discern what is the description of the problem being fixed and what is the description of the new behaviour implemented by the patch. Also this patch does not add kernel configuration file option.

Quoted 4 lines

The script has been lightly tested on the following systems:
x86_64
x86_32 (on a i686 running Debian 7 to be precise)
ARM32 (on a Yocto-based qemuarm32 ARM Versatile platform (ARM926EJ-S

cpu))

Quoted 2 lines

In the first two cases, one just has to run the script - no parameters

required.

Quoted 2 lines

In the ARM-32 case, it will, by design, fail to detect the architecture; (re)running it with the '--32-bit' and/or the

'--page-offset-32bit=<hexval>'

This does not match the code. In the code you have defined the option as '--opt-32bit'? '--32-bit' is better ;)

Quoted 1 line

option switch(es) has the desired effect: it now detects 32-bit leaking

kernel

Quoted 9 lines

addresses, if any.

Request more testing on the above and other platforms.

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

---
 scripts/leaking_addresses.pl | 190

+++++++++++++++++++++++++++++++++++--------

scripts/leaking_addresses.pl line 4

Quoted 11 lines

 1 file changed, 156 insertions(+), 34 deletions(-)

scripts/leaking_addresses.pl +0 -1

index a29e13e577a7..b0807b3a3c7c 100755
@@ -1,10 +1,10 @@
 #!/usr/bin/env perl
 #
 # (c) 2017 Tobin C. Harding <[email protected]>
-

Please make only the minimum number of changes required.

scripts/leaking_addresses.pl

Quoted 4 lines

scripts/leaking_addresses.pl +1 -1

+# (c) 2017 Kaiwan N Billimoria <[email protected]>
 # Licensed under the terms of the GNU GPL License version 2
 #
-# leaking_addresses.pl: Scan 64 bit kernel for potential leaking

addresses.

scripts/leaking_addresses.pl

Quoted 3 lines

scripts/leaking_addresses.pl +1 -0

+# leaking_addresses.pl: Scan kernel for potential leaking addresses.
 #  - Scans dmesg output.
 #  - Walks directory tree and parses each file (for each directory in

@DIRS).

scripts/leaking_addresses.pl my @DIRS = ('/proc', '/sys'); · line 35

Quoted 6 lines

scripts/leaking_addresses.pl +0 -1

 #

@@ -32,11 +32,6 @@ my @DIRS = ('/proc', '/sys');
 # Timer for parsing each file, in seconds.
 my $TIMEOUT = 10;
 
-# Script can only grep for kernel addresses on the following

architectures. If

scripts/leaking_addresses.pl my @DIRS = ('/proc', '/sys'); · line 36

Quoted 1 line

scripts/leaking_addresses.pl +0 -1

-# your architecture is not listed here and has a grep'able kernel

address please

scripts/leaking_addresses.pl my $suppress_dmesg = 0; # Don't show dmesg

Quoted 7 lines

scripts/leaking_addresses.pl +0 -3

-# consider submitting a patch.
-my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
-
 # Command line options.
 my $help = 0;
 my $debug = 0;

@@ -48,7 +43,9 @@ my $suppress_dmesg = 0;             # Don't show dmesg

in output.

scripts/leaking_addresses.pl my $suppress_dmesg = 0; # Don't show dmesg · line 43

Quoted 1 line

scripts/leaking_addresses.pl +0 -0

 my $squash_by_path = 0;              # Summary report grouped by

absolute path.

scripts/leaking_addresses.pl my $suppress_dmesg = 0; # Don't show dmesg in output. · line 51

Quoted 3 lines

scripts/leaking_addresses.pl +0 -1

 my $squash_by_filename = 0;  # Summary report grouped by filename.
 
-my $kernel_config_file = ""; # Kernel configuration file.

No need for this to be removed and re-added (if you line up the comments).

scripts/leaking_addresses.pl my $suppress_dmesg = 0; # Don't show dmesg · line 46

Quoted 1 line

scripts/leaking_addresses.pl +1 -0

+my $opt_32_bit = 0;            # Detect (only) 32-bit kernel leaking

addresses.

As previously discussed, please be consistent in naming: $opt_32bit

scripts/leaking_addresses.pl Options: · line 102

Quoted 8 lines

scripts/leaking_addresses.pl +2 -0

+my $page_offset_32bit = 0;     # 32-bit: value of CONFIG_PAGE_OFFSET.
+my $kernel_config_file = "";   # Kernel configuration file.
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',

@@ -104,10 +101,12 @@ Options:
            --squash-by-path          Show one result per unique path.
            --squash-by-filename      Show one result per unique

filename.

scripts/leaking_addresses.pl Options: · line 103

Quoted 1 line

scripts/leaking_addresses.pl +0 -0

      --kernel-config-file=<file>     Kernel configuration file (e.g

/boot/config)

scripts/leaking_addresses.pl Options: · line 104

Quoted 1 line

scripts/leaking_addresses.pl +1 -0

+     --opt-32bit                     Detect (only) 32-bit kernel

leaking addresses.

--32-bit Scan 32-bit kernel.

scripts/leaking_addresses.pl Options: · line 105

Quoted 1 line

scripts/leaking_addresses.pl +1 -0

+     --page-offset-32bit=<hex>       PAGE_OFFSET value (for 32-bit

kernels).

scripts/leaking_addresses.pl if (!$input_raw and ($squash_by_path or

Quoted 21 lines

scripts/leaking_addresses.pl +5 -3

      -d, --debug                     Display debugging output.
-     -h, --help, --versionq          Display this help and exit.
+     -h, --help, --version           Display this help and exit.
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+Scans the running kernel for potential leaking addresses.
 
 EOM
      exit($exitcode);

@@ -123,7 +122,9 @@ GetOptions(
      'squash-by-path'        => \$squash_by_path,
      'squash-by-filename'    => \$squash_by_filename,
      'raw'                   => \$raw,
-     'kernel-config-file=s'  => \$kernel_config_file,
+     'opt-32bit'             => \$opt_32_bit,
+     'page-offset-32bit=o'   => \$page_offset_32bit,
+     'kernel-config-file=s'  => \$kernel_config_file,
 ) or help(1);
 
 help(0) if ($help);

@@ -139,16 +140,15 @@ if (!$input_raw and ($squash_by_path or

$squash_by_filename)) {

scripts/leaking_addresses.pl if (!$input_raw and ($squash_by_path or $squash_by_filename)) {

Quoted 13 lines

scripts/leaking_addresses.pl +1 -8

      exit(128);
 }
 
-if (!is_supported_architecture()) {
-     printf "\nScript does not support your architecture, sorry.\n";
-     printf "\nCurrently we support: \n\n";
-     foreach(@SUPPORTED_ARCHITECTURES) {
-             printf "\t%s\n", $_;
-     }
+show_detected_architecture() if $debug;
 
-     my $archname = $Config{archname};
-     printf "\n\$ perl -MConfig -e \'print

\"\$Config{archname}\\n\"\'\n";

scripts/leaking_addresses.pl if (!$input_raw and ($squash_by_path or $squash_by_filename)) {

Quoted 3 lines

scripts/leaking_addresses.pl +2 -1

-     printf "%s\n", $archname;
+if (!is_known_architecture()) {
+     printf STDERR "\nFATAL: Script does not recognize your

architecture\n";

scripts/leaking_addresses.pl if (!$input_raw and ($squash_by_path or $squash_by_filename)) { · line 154

Quoted 8 lines

scripts/leaking_addresses.pl +5 -0

+
+     my $arch = `uname -m`;
+     chomp $arch;
+     printf "\n\$ uname -m\n";
+     printf "%s\n", $arch;
 
      exit(129);
 }

I am going to add a patch to use uname -m instead of perl -MConfig ... on powerpc. Please rebase your next version.

scripts/leaking_addresses.pl sub dprint · line 171

Quoted 6 lines

scripts/leaking_addresses.pl +1 -1

@@ -168,21 +168,45 @@ sub dprint
      printf(STDERR @_) if $debug;
 }
 
-sub is_supported_architecture
+sub is_known_architecture

Please don't change this.

Quoted 11 lines

scripts/leaking_addresses.pl +4 -3

 {
-     return (is_x86_64() or is_ppc64());
+     return (is_64bit() or is_32bit());
 }
 
-sub is_x86_64
+sub is_32bit
 {
-     my $archname = $Config{archname};
+     # 32-bit actual case
+     if (is_ix86_32()) {

scripts/leaking_addresses.pl sub dprint · line 188

Quoted 10 lines

scripts/leaking_addresses.pl +4 -1

+             return 1;
+     }
 
-     if ($archname =~ m/x86_64/) {
+     # 32-bit "forced" case (for any arch other than IA-32)
+     if ($opt_32_bit or $page_offset_32bit) {
              return 1;
      }
      return 0;
 }

Perhaps

sub is_32bit

{
        # Allow --32-bit and/or --page-offset-32bit to override.
        if ($opt_32_bit or $page_offset_32bit) {
                return 1;
        }

        if (is_ix86_32()) {
                return 1;
        }

        return 0;
}

Quoted 10 lines

scripts/leaking_addresses.pl +10 -0

+sub is_64bit
+{
+     return (is_x86_64() or is_ppc64() or is_arm64() or is_mips64());
+}
+
+sub is_x86_64
+{
+     is_arch("x86_64");
+}
+

scripts/leaking_addresses.pl sub dprint · line 209

Quoted 10 lines

scripts/leaking_addresses.pl +10 -0

+sub is_arm64
+{
+     is_arch("aarch64");
+}
+
+sub is_mips64
+{
+     is_arch("mips64");
+}
+

We cannot add these last two. There has been no talk (to my knowledge) of the
address format for mips64 or aarch64. If you have tested for these please add as a separate patch.

Quoted 25 lines

scripts/leaking_addresses.pl +18 -0

 sub is_ppc64
 {
      my $archname = $Config{archname};

@@ -193,6 +217,47 @@ sub is_ppc64
      return 0;
 }
 
+sub is_ix86_32
+{
+     my $arch = `uname -m`;
+
+     chomp $arch;
+     if ($arch =~ m/i[3456]86/) {
+             return 1;
+     }
+     return 0;
+}
+
+sub is_arch
+{
+     my ($desc) = @_;
+     my $arch = `uname -m`;
+
+     chomp $arch;
+     if ($arch eq $desc) {

scripts/leaking_addresses.pl sub is_ppc64 · line 247

Quoted 10 lines

scripts/leaking_addresses.pl +10 -0

+             return 1;
+     }
+     return 0;
+}
+
+sub show_detected_architecture
+{
+     printf "Detected architecture: ";
+     if (is_32bit()) {
+             printf "32 bit\n";

is_32bit includes the command line options so this is not _totally_ correct. Prefer

         if (is_x64_32()) {
                ...

Quoted 17 lines

scripts/leaking_addresses.pl +13 -0

+     } elsif (is_x86_64()) {
+             printf "x86_64\n";
+     } elsif (is_ppc64()) {
+             printf "PPC64\n";
+     } elsif (is_arm64()) {
+             printf "ARM64\n";
+     } elsif (is_mips64()) {
+             printf "MIPS64\n";
+     } else {
+             printf "failed to detect architecture\n"
+     }
+}
+
 # gets config option value from kernel config file
 sub get_kernel_config_option
 {

scripts/leaking_addresses.pl sub get_kernel_config_option · line 215

Quoted 4 lines

scripts/leaking_addresses.pl +0 -1

@@ -212,7 +277,6 @@ sub get_kernel_config_option
              } else {
                      @config_files = ($tmp_file);
              }
-

Please don't do white space changes in the middle or another patch.

scripts/leaking_addresses.pl sub get_kernel_config_option · line 223

Quoted 8 lines

scripts/leaking_addresses.pl +0 -1

      } else {
              my $file = '/boot/config-' . `uname -r`;
              chomp $file;

@@ -220,7 +284,6 @@ sub get_kernel_config_option
      }
 
      foreach my $file (@config_files) {
-             dprint("parsing config file: %s\n", $file);

This should be a separate patch. 'One thing per patch' is the kernel mantra

scripts/leaking_addresses.pl sub is_false_positive · line 325

Quoted 9 lines

scripts/leaking_addresses.pl +2 -0

              $value = option_from_file($option, $file);
              if ($value ne "") {
                      last;

@@ -258,6 +321,14 @@ sub is_false_positive
 {
      my ($match) = @_;
 
+     # 32 bit architectures, actual or forced
+

No need for this comment.

scripts/leaking_addresses.pl sub is_false_positive · line 331

Quoted 6 lines

scripts/leaking_addresses.pl +6 -0

+     if (is_32bit()) {
+             return is_false_positive_32bit($match);
+     }
+
+     # 64 bit architectures
+

Or this one.

Quoted 13 lines

scripts/leaking_addresses.pl +6 -0

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

@@ -281,6 +352,58 @@ sub is_in_vsyscall_memory_region
      return ($hex >= $region_min and $hex <= $region_max);
 }
 
+sub is_false_positive_32bit
+{
+     my ($match) = @_;
+     state $page_offset = get_page_offset(); # only gets called once
+
+     if ($match =~ '\b(0x)?(f|F){8}\b') {

scripts/leaking_addresses.pl sub is_in_vsyscall_memory_region · line 370

Quoted 10 lines

scripts/leaking_addresses.pl +10 -0

+             return 1;
+     }
+
+     my $addr32 = hex($match);
+     if ($addr32 < $page_offset) {
+             return 1;
+     }
+
+     return 0;
+}

The return value of this sub routine is confusing. Either it should be commented as returning a hex value or it should return a string. I prefer a string since the rest of the subs in the file return strings, less cognitive load to keep everything the same. Unless there is some clear advantage to returning a hex value (which I cannot see).

Quoted 4 lines

scripts/leaking_addresses.pl +4 -0

+sub get_page_offset
+{
+     my $page_offset;
+     my $default_offset = hex("0xc0000000");

scripts/leaking_addresses.pl sub is_in_vsyscall_memory_region · line 385

Quoted 10 lines

scripts/leaking_addresses.pl +10 -0

+     my @config_files;
+
+     # Allow --page-offset-32bit to override.
+     if ($page_offset_32bit != 0) {
+             return $page_offset_32bit;
+     }
+
+     $page_offset = get_kernel_config_option('CONFIG_PAGE_OFFSET');
+     return $default_offset;
+}

This is wrong.

Quoted 11 lines

scripts/leaking_addresses.pl +11 -0

+
+sub parse_kernel_config_file
+{
+     my ($file) = @_;
+     my $config = 'CONFIG_PAGE_OFFSET';
+     my $str = "";
+     my $val = "";
+
+     open(my $fh, "<", $file) or return "";
+     while (my $line = <$fh> ) {
+             if ($line =~ /^$config/) {

scripts/leaking_addresses.pl sub is_in_vsyscall_memory_region · line 406

Quoted 10 lines

scripts/leaking_addresses.pl +10 -0

+                     ($str, $val) = split /=/, $line;
+                     chomp($val);
+                     last;
+             }
+     }
+
+     close $fh;
+     return $val;
+}
+

When do you call this subroutine?

scripts/leaking_addresses.pl sub may_leak_address · line 304

Quoted 9 lines

scripts/leaking_addresses.pl +0 -2

 # True if argument potentially contains a kernel address.
 sub may_leak_address
 {

@@ -300,8 +423,6 @@ sub may_leak_address
      }
 
      $address_re = get_address_re();
-     dprint("Kernel address regular expression: %s\n", $address_re);
-

Please see comments above on 'one thing per patch'.

Quoted 8 lines

scripts/leaking_addresses.pl +0 -1

      while (/($address_re)/g) {
              if (!is_false_positive($1)) {
                      return 1;

@@ -313,16 +434,17 @@ sub may_leak_address
 
 sub get_address_re
 {
-     my $re;

scripts/leaking_addresses.pl sub may_leak_address · line 325

Quoted 10 lines

scripts/leaking_addresses.pl +1 -4

+     my $re = "";
 
      if (is_x86_64()) {
              $re = get_x86_64_re();
      } elsif (is_ppc64()) {
              $re = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
-     }
-
-     if ($re eq "") {
-             print STDERR "$0: failed to build kernel address regular

expression\n";

scripts/leaking_addresses.pl sub may_leak_address · line 450

Quoted 8 lines

scripts/leaking_addresses.pl +5 -0

+     ###
+     # Any special cases for other arch's go below this line
+     ###
+     } else {  # nothing? then we assume it's a generic 32-bit
+             $re = '\b(0x)?[[:xdigit:]]{8}\b';
      }
 
      return $re;

This is messy. We definitely want to warn if we cannot fully determine the correct regex to use. Also, if you would like to default to 32-bit you will need to back it up with some reasoning. Currently there is no default if the correct regex cannot be ascertained. At this stage I feel this is the correct behaviour, I'm open to discussion though.

Quoted 2 lines

--
2.14.3

thanks,
Tobin.

Hi Tobin,

Thanks very much for your detailed review.
Just wanted to say that am up to my neck in work (an exceptionally busy time), hence will take a while to work on this - around another 3 weeks perhaps.
I'd like to continue, but if you feel it's too long please move ahead.