2017-12-07 04:32:38

by Tobin C. Harding

[permalink] [raw]
Subject: [PATCH 0/5] leaking_addresses: support 5 page table level

This set adds support for 5 page table levels on x86_64.

Patch 1 and 2 are clean up.

Patch 3 adds checking vsyscall memory region instead of just first and
last address.

Patch 4 adds support for locating and parsing the current kernel config
file. It feels like this may be re-inventing the wheel (also the
implementation is not as thorough as scripts/extract-ikconfig?) Much of
the code for this has been posted on LKML a few times now by Kaiwan
Billimoria. As such, it uses the new Co-Developed-by tag.

Patch 5 adds the 5 page table levels stuff. It is trivial once we have a
value from CONFIG_PGTABLE_LEVELS. Just set the regular expression
accordingly.

Perl tips, and code style tips in general, most appreciated.

As usual, any kernel development protocol tips also most
appreciated. Both Kaiwan and myself are pretty new around here, feel
free to give us a kick if we need it.

thanks,
Tobin.

Tobin C. Harding (5):
leaking_addresses: remove command examples
leaking_addresses: indent dependant options
leaking_addresses: add range check for vsyscall memory
leaking_addresses: add support for kernel config file
leaking_addresses: add support for 5 page table levels

scripts/leaking_addresses.pl | 169 +++++++++++++++++++++++++++++++++++--------
1 file changed, 137 insertions(+), 32 deletions(-)

--
2.7.4


2017-12-07 04:32:42

by Tobin C. Harding

[permalink] [raw]
Subject: [PATCH 1/5] leaking_addresses: remove command examples

Currently help output includes command examples. These were cute when we
first started development of this script but are unnecessary.

Remove command examples.

Signed-off-by: Tobin C. Harding <[email protected]>
---
scripts/leaking_addresses.pl | 11 -----------
1 file changed, 11 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 1c5637db82a5..2c5da2296e94 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -102,17 +102,6 @@ Options:
-d, --debug Display debugging output.
-h, --help, --version Display this help and exit.

-Examples:
-
- # Scan kernel and dump raw results.
- $0
-
- # Scan kernel and save results to file.
- $0 --output-raw scan.out
-
- # View summary report.
- $0 --input-raw scan.out --squash-by-filename
-
Scans the running (64 bit) kernel for potential leaking addresses.

EOM
--
2.7.4

2017-12-07 04:32:48

by Tobin C. Harding

[permalink] [raw]
Subject: [PATCH 2/5] leaking_addresses: indent dependant options

A number of the command line options to script are dependant on the
option --input-raw being set. If we indent these options it makes
explicit this dependency.

Indent options dependant on --raw-input.

Signed-off-by: Tobin C. Harding <[email protected]>
---
scripts/leaking_addresses.pl | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 2c5da2296e94..066c609b1adb 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -93,14 +93,14 @@ Version: $V

Options:

- -o, --output-raw=<file> Save results for future processing.
- -i, --input-raw=<file> Read results from file instead of scanning.
- --raw Show raw results (default).
- --suppress-dmesg Do not show dmesg results.
- --squash-by-path Show one result per unique path.
- --squash-by-filename Show one result per unique filename.
- -d, --debug Display debugging output.
- -h, --help, --version Display this help and exit.
+ -o, --output-raw=<file> Save results for future processing.
+ -i, --input-raw=<file> Read results from file instead of scanning.
+ --raw Show raw results (default).
+ --suppress-dmesg Do not show dmesg results.
+ --squash-by-path Show one result per unique path.
+ --squash-by-filename Show one result per unique filename.
+ -d, --debug Display debugging output.
+ -h, --help, --versionq Display this help and exit.

Scans the running (64 bit) kernel for potential leaking addresses.

--
2.7.4

2017-12-07 04:32:55

by Tobin C. Harding

[permalink] [raw]
Subject: [PATCH 4/5] leaking_addresses: add support for kernel config file

Features that rely on the ability to get kernel configuration options
are ready to be implemented in script. In preparation for this we can
add support for kernel config options as a separate patch to ease
review.

Add support for locating and parsing kernel configuration file.

Signed-off-by: Tobin C. Harding <[email protected]>
Co-Developed-by: Kaiwan N Billimoria <[email protected]>
---

get_kernel_config_option() is not super clean, any improvements most welcome.

Kaiwan,

This needs your Signed-off-by tag if you want me to apply it with
the Co-Developed-tag

thanks,
Tobin.

scripts/leaking_addresses.pl | 64 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index cb69ccd4153a..892bfe9e01fe 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -42,10 +42,10 @@ my $debug = 0;
my $raw = 0;
my $output_raw = ""; # Write raw results to file.
my $input_raw = ""; # Read raw results from file instead of scanning.
-
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.

# Do not parse these files (absolute path).
my @skip_parse_files_abs = ('/proc/kmsg',
@@ -100,6 +100,7 @@ Options:
--suppress-dmesg Do not show dmesg results.
--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)
-d, --debug Display debugging output.
-h, --help, --versionq Display this help and exit.

@@ -119,6 +120,7 @@ GetOptions(
'squash-by-path' => \$squash_by_path,
'squash-by-filename' => \$squash_by_filename,
'raw' => \$raw,
+ 'kernel-config-file=s' => \$kernel_config_file,
) or help(1);

help(0) if ($help);
@@ -188,6 +190,66 @@ sub is_ppc64
return 0;
}

+# gets config option value from kernel config file
+sub get_kernel_config_option
+{
+ my ($option) = @_;
+ my $value = "";
+ my $tmp_file = "";
+ my @config_files;
+
+ # Allow --kernel-config-file to override.
+ if ($kernel_config_file ne "") {
+ @config_files = ($kernel_config_file);
+ } elsif (-R "/proc/config.gz") {
+ my $tmp_file = "/tmp/tmpkconf";
+
+ if (system("gunzip < /proc/config.gz > $tmp_file")) {
+ dprint "$0: system(gunzip < /proc/config.gz) failed\n";
+ } else {
+ @config_files = ($tmp_file);
+ }
+
+ } else {
+ my $file = '/boot/config-' . `uname -r`;
+ @config_files = ($file, '/boot/config');
+ }
+
+ foreach my $file (@config_files) {
+# chomp $config_file;
+ $value = option_from_file($option, $file);
+ if ($value ne "") {
+ last;
+ }
+ }
+
+ if ($tmp_file ne "") {
+ system("rm -f $tmp_file");
+ }
+
+ return $value;
+}
+
+# Parses $file and returns kernel configuration option value.
+sub option_from_file
+{
+ my ($option, $file) = @_;
+ my $str = "";
+ my $val = "";
+
+ open(my $fh, "<", $file) or return "";
+ while (my $line = <$fh> ) {
+ if ($line =~ /^$option/) {
+ ($str, $val) = split /=/, $line;
+ chomp($val);
+ last;
+ }
+ }
+
+ close $fh;
+ return $val;
+}
+
sub is_false_positive
{
my ($match) = @_;
--
2.7.4

2017-12-07 04:33:08

by Tobin C. Harding

[permalink] [raw]
Subject: [PATCH 5/5] leaking_addresses: add support for 5 page table levels

Currently script only supports 4 page table levels because of the way
the kernel address regular expression is crafted. We can do better than
this. Using previously added support for kernel configuration options we
can get the number of page table levels defined by
CONFIG_PGTABLE_LEVELS. Using this value a correct regular expression can
be crafted. This only supports 5 page tables on x86_64.

Add support for 5 page table levels on x86_64.

Signed-off-by: Tobin C. Harding <[email protected]>
---
scripts/leaking_addresses.pl | 60 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 892bfe9e01fe..82be5f18ea5f 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -21,6 +21,7 @@ use Term::ANSIColor qw(:constants);
use Getopt::Long qw(:config no_auto_abbrev);
use Config;
use bigint qw/hex/;
+use feature 'state';

my $P = $0;
my $V = '0.01';
@@ -39,12 +40,14 @@ my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
# Command line options.
my $help = 0;
my $debug = 0;
+
my $raw = 0;
my $output_raw = ""; # Write raw results to file.
my $input_raw = ""; # Read raw results from file instead of scanning.
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.

# Do not parse these files (absolute path).
@@ -212,11 +215,13 @@ sub get_kernel_config_option

} else {
my $file = '/boot/config-' . `uname -r`;
+ chomp $file;
@config_files = ($file, '/boot/config');
}

foreach my $file (@config_files) {
-# chomp $config_file;
+ printf("file: %s\n", $file);
+
$value = option_from_file($option, $file);
if ($value ne "") {
last;
@@ -295,12 +300,8 @@ sub may_leak_address
return 0;
}

- # One of these is guaranteed to be true.
- if (is_x86_64()) {
- $address_re = '\b(0x)?ffff[[:xdigit:]]{12}\b';
- } elsif (is_ppc64()) {
- $address_re = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
- }
+ $address_re = get_address_re();
+ dprint("Kernel address regular expression: %s\n", $address_re);

while (/($address_re)/g) {
if (!is_false_positive($1)) {
@@ -311,6 +312,51 @@ sub may_leak_address
return 0;
}

+sub get_address_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";
+ }
+
+ return $re;
+}
+
+sub get_x86_64_re
+{
+ state $ptl = get_page_table_levels();
+ my $re;
+
+ if ($ptl == 5) {
+ $re = '\b(0x)?ff[[:xdigit:]]{14}\b';
+ } else {
+ $re = '\b(0x)?ffff[[:xdigit:]]{12}\b';
+ }
+
+ return $re;
+}
+
+sub get_page_table_levels
+{
+ my $ptl = "";
+ my $default_ptl = "4";
+
+ $ptl = get_kernel_config_option('CONFIG_PGTABLE_LEVELS');
+ if ($ptl eq "") {
+ $ptl = $default_ptl;
+ printf(STDERR "$0: defaulting to %s page table levels\n", $default_ptl);
+ }
+
+ return $ptl;
+}
+
sub parse_dmesg
{
open my $cmd, '-|', 'dmesg';
--
2.7.4

2017-12-07 04:33:24

by Tobin C. Harding

[permalink] [raw]
Subject: [PATCH 3/5] leaking_addresses: add range check for vsyscall memory

Currently script checks only first and last address in the vsyscall
memory range. We can do better than this.

When checking for false positives against $match, convert $match to
a hexadecimal value then check if it lies within the range of vsyscall
addresses.

Signed-off-by: Tobin C. Harding <[email protected]>
---
scripts/leaking_addresses.pl | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 066c609b1adb..cb69ccd4153a 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -20,6 +20,7 @@ use Cwd 'abs_path';
use Term::ANSIColor qw(:constants);
use Getopt::Long qw(:config no_auto_abbrev);
use Config;
+use bigint qw/hex/;

my $P = $0;
my $V = '0.01';
@@ -196,17 +197,24 @@ sub is_false_positive
return 1;
}

- if (is_x86_64()) {
- # vsyscall memory region, we should probably check against a range here.
- if ($match =~ '\bf{10}600000\b' or
- $match =~ '\bf{10}601000\b') {
- return 1;
- }
+ if (is_x86_64() and is_in_vsyscall_memory_region($match)) {
+ return 1;
}

return 0;
}

+sub is_in_vsyscall_memory_region
+{
+ my ($match) = @_;
+
+ my $hex = hex($match);
+ my $region_min = hex("0xffffffffff600000");
+ my $region_max = hex("0xffffffffff601000");
+
+ return ($hex >= $region_min and $hex <= $region_max);
+}
+
# True if argument potentially contains a kernel address.
sub may_leak_address
{
--
2.7.4

2017-12-08 01:55:58

by Kaiwan N Billimoria

[permalink] [raw]
Subject: Re: [PATCH 4/5] leaking_addresses: add support for kernel config file

On Thu, Dec 7, 2017 at 10:02 AM, Tobin C. Harding <[email protected]> wrote:
> Features that rely on the ability to get kernel configuration options
> are ready to be implemented in script. In preparation for this we can
> add support for kernel config options as a separate patch to ease
> review.
>
> Add support for locating and parsing kernel configuration file.
>
> Signed-off-by: Tobin C. Harding <[email protected]>
> Co-Developed-by: Kaiwan N Billimoria <[email protected]>
> ---
>
> get_kernel_config_option() is not super clean, any improvements most welcome.
>
> Kaiwan,
>
> This needs your Signed-off-by tag if you want me to apply it with
> the Co-Developed-tag
>
> thanks,
> Tobin.
>
Adding my signed-off tag..

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

> scripts/leaking_addresses.pl | 64 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index cb69ccd4153a..892bfe9e01fe 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -42,10 +42,10 @@ my $debug = 0;
> my $raw = 0;
> my $output_raw = ""; # Write raw results to file.
> my $input_raw = ""; # Read raw results from file instead of scanning.
> -
> 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.
>
> # Do not parse these files (absolute path).
> my @skip_parse_files_abs = ('/proc/kmsg',
> @@ -100,6 +100,7 @@ Options:
> --suppress-dmesg Do not show dmesg results.
> --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)
> -d, --debug Display debugging output.
> -h, --help, --versionq Display this help and exit.
>
> @@ -119,6 +120,7 @@ GetOptions(
> 'squash-by-path' => \$squash_by_path,
> 'squash-by-filename' => \$squash_by_filename,
> 'raw' => \$raw,
> + 'kernel-config-file=s' => \$kernel_config_file,
> ) or help(1);
>
> help(0) if ($help);
> @@ -188,6 +190,66 @@ sub is_ppc64
> return 0;
> }
>
> +# gets config option value from kernel config file
> +sub get_kernel_config_option
> +{
> + my ($option) = @_;
> + my $value = "";
> + my $tmp_file = "";
> + my @config_files;
> +
> + # Allow --kernel-config-file to override.
> + if ($kernel_config_file ne "") {
> + @config_files = ($kernel_config_file);
> + } elsif (-R "/proc/config.gz") {
> + my $tmp_file = "/tmp/tmpkconf";
> +
> + if (system("gunzip < /proc/config.gz > $tmp_file")) {
> + dprint "$0: system(gunzip < /proc/config.gz) failed\n";
> + } else {
> + @config_files = ($tmp_file);
> + }
> +
> + } else {
> + my $file = '/boot/config-' . `uname -r`;
> + @config_files = ($file, '/boot/config');
> + }
> +
> + foreach my $file (@config_files) {
> +# chomp $config_file;
> + $value = option_from_file($option, $file);
> + if ($value ne "") {
> + last;
> + }
> + }
> +
> + if ($tmp_file ne "") {
> + system("rm -f $tmp_file");
> + }
> +
> + return $value;
> +}
> +
> +# Parses $file and returns kernel configuration option value.
> +sub option_from_file
> +{
> + my ($option, $file) = @_;
> + my $str = "";
> + my $val = "";
> +
> + open(my $fh, "<", $file) or return "";
> + while (my $line = <$fh> ) {
> + if ($line =~ /^$option/) {
> + ($str, $val) = split /=/, $line;
> + chomp($val);
> + last;
> + }
> + }
> +
> + close $fh;
> + return $val;
> +}
> +
> sub is_false_positive
> {
> my ($match) = @_;
> --
> 2.7.4
>