[PATCH 0/7] scripts/leaking_addresses: add summary report

[PATCH 0/7] scripts/leaking_addresses: add summary report

[Tobin C. Harding]

This series includes changes submitted as

[PATCH v4 ] scripts: add leaking_addresses.pl

before I realize Linus had merged v3.

Does some clean up and implements changes suggested by Petr Mladek and Kees
Cook.

Adds printing a summary report instead of raw results as suggested by Linus
Torvalds.

thanks,
Tobin.

Tobin C. Harding (7):
  scripts/leaking_addresses: use tabs not spaces
  scripts/leaking_addresses: remove dead code
  scripts/leaking_addresses: remove command line options
  scripts/leaking_addresses: add reporting
  scripts/leaking_addresses: add emailing results
  scripts/leaking_addresses: fix comment typo
  scripts/leaking_addresses: don't parse usbmon

 scripts/leaking_addresses.pl | 351 ++++++++++++++++++++++++++++++-------------
 1 file changed, 250 insertions(+), 101 deletions(-)
 mode change 100755 => 100644 scripts/leaking_addresses.pl

-- 
2.7.4

[PATCH 1/7] scripts/leaking_addresses: use tabs not spaces

[Tobin C. Harding]

Current code uses spaces instead of tabs in places.

Use tabs instead of spaces.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 54 ++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 2977371b2956..b64efcecbb5e 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -170,46 +170,46 @@ sub push_to_global
 
 sub is_false_positive
 {
-        my ($match) = @_;
+	my ($match) = @_;
+
+	if ($match =~ '\b(0x)?(f|F){16}\b' or
+	    $match =~ '\b(0x)?0{16}\b') {
+		return 1;
+	}
 
-        if ($match =~ '\b(0x)?(f|F){16}\b' or
-            $match =~ '\b(0x)?0{16}\b') {
-                return 1;
-        }
 
-        # 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 ($match =~ '\bf{10}600000\b' or# vsyscall memory region, we should probably check against a range here.
+	    $match =~ '\bf{10}601000\b') {
+		return 1;
+	}
 
-        return 0;
+	return 0;
 }
 
 # True if argument potentially contains a kernel address.
 sub may_leak_address
 {
-        my ($line) = @_;
-        my $address = '\b(0x)?ffff[[:xdigit:]]{12}\b';
+	my ($line) = @_;
+	my $address = '\b(0x)?ffff[[:xdigit:]]{12}\b';
 
-        # Signal masks.
-        if ($line =~ '^SigBlk:' or
-            $line =~ '^SigCgt:') {
-                return 0;
-        }
+	# Signal masks.
+	if ($line =~ '^SigBlk:' or
+	    $line =~ '^SigCgt:') {
+		return 0;
+	}
 
-        if ($line =~ '\bKEY=[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b' or
-            $line =~ '\b[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b') {
+	if ($line =~ '\bKEY=[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b' or
+	    $line =~ '\b[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b') {
 		return 0;
-        }
+	}
 
-        while (/($address)/g) {
-                if (!is_false_positive($1)) {
-                        return 1;
-                }
-        }
+	while (/($address)/g) {
+		if (!is_false_positive($1)) {
+			return 1;
+		}
+	}
 
-        return 0;
+	return 0;
 }
 
 sub parse_dmesg
-- 
2.7.4

[PATCH 2/7] scripts/leaking_addresses: remove dead code

[Tobin C. Harding]

debug_arrays is not called. Also, %seen hash is not used. We should
remove unused code.

Remove dead code.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index b64efcecbb5e..94b22d5b9237 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -133,14 +133,6 @@ walk(@DIRS);
 
 exit 0;
 
-sub debug_arrays
-{
-	print 'dirs_any: ' . join(", ", @skip_walk_dirs_any) . "\n";
-	print 'dirs_abs: ' . join(", ", @skip_walk_dirs_abs) . "\n";
-	print 'parse_any: ' . join(", ", @skip_parse_files_any) . "\n";
-	print 'parse_abs: ' . join(", ", @skip_parse_files_abs) . "\n";
-}
-
 sub dprint
 {
 	printf(STDERR @_) if $debug;
@@ -281,7 +273,6 @@ sub skip_walk
 sub walk
 {
 	my @dirs = @_;
-	my %seen;
 
 	while (my $pwd = shift @dirs) {
 		next if (skip_walk($pwd));
-- 
2.7.4

[PATCH 3/7] scripts/leaking_addresses: remove command line options

[Tobin C. Harding]

Currently script accepts files to skip. This was added to make running
the script faster (for repeat runs). We can remove this functionality in
preparation for adding sub commands (scan and format) to the script.

Remove command line options.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 58 --------------------------------------------
 1 file changed, 58 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 94b22d5b9237..719ed0aaede7 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -7,25 +7,6 @@
 #  - Scans dmesg output.
 #  - Walks directory tree and parses each file (for each directory in @DIRS).
 #
-# You can configure the behaviour of the script;
-#
-#  - By adding paths, for directories you do not want to walk;
-#     absolute paths: @skip_walk_dirs_abs
-#     directory names: @skip_walk_dirs_any
-#
-#  - By adding paths, for files you do not want to parse;
-#     absolute paths: @skip_parse_files_abs
-#     file names: @skip_parse_files_any
-#
-# The use of @skip_xxx_xxx_any causes files to be skipped where ever they occur.
-# For example adding 'fd' to @skip_walk_dirs_any causes the fd/ directory to be
-# skipped for all PID sub-directories of /proc
-#
-# The same thing can be achieved by passing command line options to --dont-walk
-# and --dont-parse. If absolute paths are supplied to these options they are
-# appended to the @skip_xxx_xxx_abs arrays. If file names are supplied to these
-# options, they are appended to the @skip_xxx_xxx_any arrays.
-#
 # Use --debug to output path before parsing, this is useful to find files that
 # cause the script to choke.
 #
@@ -50,8 +31,6 @@ my @DIRS = ('/proc', '/sys');
 # Command line options.
 my $help = 0;
 my $debug = 0;
-my @dont_walk = ();
-my @dont_parse = ();
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',
@@ -96,20 +75,9 @@ Version: $V
 
 Options:
 
-	--dont-walk=<dir>      Don't walk tree starting at <dir>.
-	--dont-parse=<file>    Don't parse <file>.
 	-d, --debug                Display debugging output.
 	-h, --help, --version      Display this help and exit.
 
-If an absolute path is passed to --dont_XXX then this path is skipped. If a
-single filename is passed then this file/directory will be skipped when
-appearing under any subdirectory.
-
-Example:
-
-	# Just scan dmesg output.
-	scripts/leaking_addresses.pl --dont_walk_abs /proc --dont_walk_abs /sys
-
 Scans the running (64 bit) kernel for potential leaking addresses.
 
 EOM
@@ -117,8 +85,6 @@ EOM
 }
 
 GetOptions(
-	'dont-walk=s'		=> \@dont_walk,
-	'dont-parse=s'		=> \@dont_parse,
 	'd|debug'		=> \$debug,
 	'h|help'		=> \$help,
 	'version'		=> \$help
@@ -126,8 +92,6 @@ GetOptions(
 
 help(0) if ($help);
 
-push_to_global();
-
 parse_dmesg();
 walk(@DIRS);
 
@@ -138,28 +102,6 @@ sub dprint
 	printf(STDERR @_) if $debug;
 }
 
-sub push_in_abs_any
-{
-	my ($in, $abs, $any) = @_;
-
-	foreach my $path (@$in) {
-		if (File::Spec->file_name_is_absolute($path)) {
-			push @$abs, $path;
-		} elsif (index($path,'/') == -1) {
-			push @$any, $path;
-		} else {
-			print 'path error: ' . $path;
-		}
-	}
-}
-
-# Push command line options to global arrays.
-sub push_to_global
-{
-	push_in_abs_any(\@dont_walk, \@skip_walk_dirs_abs, \@skip_walk_dirs_any);
-	push_in_abs_any(\@dont_parse, \@skip_parse_files_abs, \@skip_parse_files_any);
-}
-
 sub is_false_positive
 {
 	my ($match) = @_;
-- 
2.7.4

[PATCH 4/7] scripts/leaking_addresses: add reporting

[Tobin C. Harding]

Currently script just dumps all results found. Potentially, this risks
loosing single results among multiple duplicate results. We need some
way of restricting duplicates to assist users of the script. It would
also be nice if we got a report instead of raw results.

Duplicates can be defined in various ways, instead of trying to find a
single perfect solution we can present the user with various options to
display the output. Doing so will typically lead to users wanting to
view the output multiple times. Currently we scan the kernel each time,
this is slow and unnecessary. We can expedite the process by writing the
results to file for subsequent viewing.

Add sub-commands `scan` and `format`. Display output as a report instead
of raw results. Add --raw flag to view raw results. Save results to
file. For subsequent calls to `format` parse output file instead of
re-scanning.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 201 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 187 insertions(+), 14 deletions(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 719ed0aaede7..4c31e935319b 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -21,14 +21,19 @@ use File::Spec;
 use Cwd 'abs_path';
 use Term::ANSIColor qw(:constants);
 use Getopt::Long qw(:config no_auto_abbrev);
+use File::Spec::Functions 'catfile';
 
 my $P = $0;
 my $V = '0.01';
 
-# Directories to scan.
+# Directories to scan (we scan `dmesg` also).
 my @DIRS = ('/proc', '/sys');
 
 # Command line options.
+my $output = "scan.out";
+my $suppress_dmesg = 0;
+my $squash_by_path = 0;
+my $raw = 0;
 my $help = 0;
 my $debug = 0;
 
@@ -70,21 +75,34 @@ sub help
 	my ($exitcode) = @_;
 
 	print << "EOM";
-Usage: $P [OPTIONS]
+
+Usage: $P COMMAND [OPTIONS]
 Version: $V
 
+Commands:
+
+	scan	Scan the kernel (savesg raw results to file and runs `format`).
+	format	Parse results file and format output.
+
 Options:
 
-	-d, --debug                Display debugging output.
-	-h, --help, --version      Display this help and exit.
+	-o, --output=<file>	 Raw results output file, used for later formatting.
+	    --suppress-dmesg	 Do not show dmesg results.
+	    --squash-by-path	 Show one result per unique path.
+	    --raw	 	 Show raw results.
+	-d, --debug              Display debugging output.
+	-h, --help, --version    Display this help and exit.
 
 Scans the running (64 bit) kernel for potential leaking addresses.
-
 EOM
 	exit($exitcode);
 }
 
 GetOptions(
+	'o|output=s'		=> \$output,
+	'suppress-dmesg'	=> \$suppress_dmesg,
+	'squash-by-path'	=> \$squash_by_path,
+	'raw'			=> \$raw,
 	'd|debug'		=> \$debug,
 	'h|help'		=> \$help,
 	'version'		=> \$help
@@ -92,8 +110,21 @@ GetOptions(
 
 help(0) if ($help);
 
-parse_dmesg();
-walk(@DIRS);
+my ($command) = @ARGV;
+if (not defined $command) {
+	help(128);
+}
+
+if ($command ne 'scan' and $command ne 'format') {
+	printf "\nUnknown command: %s\n\n", $command;
+	help(128);
+}
+
+if ($command eq 'scan') {
+	scan();
+}
+
+format_output();
 
 exit 0;
 
@@ -102,6 +133,17 @@ sub dprint
 	printf(STDERR @_) if $debug;
 }
 
+sub scan
+{
+	open (my $fh, '>', "$output") or die "$0: $output: $!\n";
+	select $fh;
+
+	parse_dmesg();
+	walk(@DIRS);
+
+	select STDOUT;
+}
+
 sub is_false_positive
 {
 	my ($match) = @_;
@@ -120,30 +162,39 @@ sub is_false_positive
 	return 0;
 }
 
-# True if argument potentially contains a kernel address.
 sub may_leak_address
 {
 	my ($line) = @_;
+
+	my @addresses = extract_addresses($line);
+	return @addresses > 0;
+}
+
+# Return _all_ non false positive addresses from $line.
+sub extract_addresses
+{
+	my ($line) = @_;
 	my $address = '\b(0x)?ffff[[:xdigit:]]{12}\b';
+	my (@addresses, @empty);
 
 	# Signal masks.
 	if ($line =~ '^SigBlk:' or
 	    $line =~ '^SigCgt:') {
-		return 0;
+		return @empty;
 	}
 
 	if ($line =~ '\bKEY=[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b' or
 	    $line =~ '\b[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b') {
-		return 0;
+		return @empty;
 	}
 
-	while (/($address)/g) {
+	while ($line =~ /($address)/g) {
 		if (!is_false_positive($1)) {
-			return 1;
+			push @addresses, $1;
 		}
 	}
 
-	return 0;
+	return @addresses;
 }
 
 sub parse_dmesg
@@ -203,7 +254,6 @@ sub parse_file
 	close $fh;
 }
 
-
 # True if we should skip walking this directory.
 sub skip_walk
 {
@@ -236,3 +286,126 @@ sub walk
 		}
 	}
 }
+
+sub format_output
+{
+	if ($raw) {
+		dump_raw_output();
+		return;
+	}
+
+	my ($total, $dmesg, $paths, $files) = parse_raw_file();
+
+	printf "\nTotal number of results from scan (incl dmesg): %d\n", $total;
+
+	if (!$suppress_dmesg) {
+		print_dmesg($dmesg);
+	}
+	squash_by($files, 'filename');
+
+	if ($squash_by_path) {
+		squash_by($paths, 'path');
+	}
+}
+
+sub dump_raw_output
+{
+	open (my $fh, '<', $output) or die "$0: $output: $!\n";
+	while (<$fh>) {
+		print $_;
+	}
+	close $fh;
+}
+
+sub print_dmesg
+{
+	my ($dmesg) = @_;
+
+	print "\ndmesg output:\n";
+
+	if (@$dmesg == 0) {
+		print "<no results>\n";
+		return;
+	}
+
+	foreach(@$dmesg) {
+		my $index = index($_, ': ');
+		$index += 2;    # skid ': '
+		print substr($_, $index);
+	}
+}
+
+sub squash_by
+{
+	my ($ref, $desc) = @_;
+
+	print "\nResults squashed by $desc (excl dmesg). ";
+	print "Displaying [<number of results> <$desc>], <example result>\n";
+
+	if (keys %$ref == 0) {
+		print "<no results>\n";
+		return;
+	}
+
+	foreach(keys %$ref) {
+		my $lines = $ref->{$_};
+		my $length = @$lines;
+		printf "[%d %s] %s", $length, $_, @$lines[0];
+	}
+}
+
+sub parse_raw_file
+{
+	my $total = 0;          # Total number of lines parsed.
+	my @dmesg;              # dmesg output.
+	my %files;              # Unique filenames containing leaks.
+	my %paths;              # Unique paths containing leaks.
+
+	open (my $fh, '<', $output) or die "$0: $output: $!\n";
+	while (my $line = <$fh>) {
+		$total++;
+
+		if ("dmesg:" eq substr($line, 0, 6)) {
+			push @dmesg, $line;
+			next;
+		}
+
+		cache_path(\%paths, $line);
+		cache_filename(\%files, $line);
+	}
+
+	return $total, \@dmesg, \%paths, \%files;
+}
+
+sub cache_path
+{
+	my ($paths, $line) = @_;
+
+	my $index = index($line, ': ');
+	my $path = substr($line, 0, $index);
+
+	$index += 2;            # skip ': '
+	add_to_cache($paths, $path, substr($line, $index));
+}
+
+sub cache_filename
+{
+	my ($files, $line) = @_;
+
+	my $index = index($line, ': ');
+	my $path = substr($line, 0, $index);
+	my $filename = basename($path);
+
+	$index += 2;            # skip ': '
+	add_to_cache($files, $filename, substr($line, $index));
+}
+
+sub add_to_cache
+{
+	my ($cache, $key, $value) = @_;
+
+	if (!$cache->{$key}) {
+		$cache->{$key} = ();
+	}
+	push @{$cache->{$key}}, $value;
+}
-- 
2.7.4

Re: [PATCH 4/7] scripts/leaking_addresses: add reporting

[Petr Mladek]

On Wed 2017-11-08 14:37:36, Tobin C. Harding wrote:
> Currently script just dumps all results found. Potentially, this risks
> loosing single results among multiple duplicate results. We need some
> way of restricting duplicates to assist users of the script. It would
> also be nice if we got a report instead of raw results.
> 
> Duplicates can be defined in various ways, instead of trying to find a
> single perfect solution we can present the user with various options to
> display the output. Doing so will typically lead to users wanting to
> view the output multiple times. Currently we scan the kernel each time,
> this is slow and unnecessary. We can expedite the process by writing the
> results to file for subsequent viewing.
> 
> Add sub-commands `scan` and `format`. Display output as a report instead
> of raw results. Add --raw flag to view raw results. Save results to
> file. For subsequent calls to `format` parse output file instead of
> re-scanning.
> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  scripts/leaking_addresses.pl | 201 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 187 insertions(+), 14 deletions(-)
> 
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 719ed0aaede7..4c31e935319b 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -21,14 +21,19 @@ use File::Spec;
>  use Cwd 'abs_path';
>  use Term::ANSIColor qw(:constants);
>  use Getopt::Long qw(:config no_auto_abbrev);
> +use File::Spec::Functions 'catfile';
>  
>  my $P = $0;
>  my $V = '0.01';
>  
> -# Directories to scan.
> +# Directories to scan (we scan `dmesg` also).
>  my @DIRS = ('/proc', '/sys');
>  
>  # Command line options.
> +my $output = "scan.out";

The hard-coded filename and its use is dangerous. Nobody expects that
this kind of script writes/re-writes a file on the system.

> +my $suppress_dmesg = 0;
> +my $squash_by_path = 0;
> +my $raw = 0;
>  my $help = 0;
>  my $debug = 0;
>  
> @@ -70,21 +75,34 @@ sub help
>  	my ($exitcode) = @_;
>  
>  	print << "EOM";
> -Usage: $P [OPTIONS]
> +
> +Usage: $P COMMAND [OPTIONS]
>  Version: $V
>  
> +Commands:
> +
> +	scan	Scan the kernel (savesg raw results to file and runs `format`).
> +	format	Parse results file and format output.

The later formatting/filtering might be useful but the use
of the hard coded file is strange. Also it is pity that
the script does not do anything useful out of box.

I suggest to remove the commands and do the scan out of box.
It should not store anything on the disk by default.

Then we could define following options:

    -o, --output=<file>	 Store raw results into file for later formatting.
    -i, --input=<file>   Read raw result from file and just format them.

Well, it is still somehow non-intuitive. It might help to
be more explicit:

    -o, --output-raw=<file>
    -i, --input-raw=<file>


>  Options:
>  
> -	-d, --debug                Display debugging output.
> -	-h, --help, --version      Display this help and exit.
> +	-o, --output=<file>	 Raw results output file, used for later formatting.
> +	    --suppress-dmesg	 Do not show dmesg results.
> +	    --squash-by-path	 Show one result per unique path.

I would personally add also option for the default mode:

	    --squash-by-filename Show one result per unique filename
				 (default).

In fact, I would personally use --squash-by-path or even --raw by
default. These provide easy to understand output. While the
--squash-by-filename mode has pretty good results but
it is quite non-intuitive.

Best Regards,
Petr

Re: [PATCH 4/7] scripts/leaking_addresses: add reporting

[Tobin C. Harding]

On Wed, Nov 08, 2017 at 11:42:21AM +0100, Petr Mladek wrote:
> On Wed 2017-11-08 14:37:36, Tobin C. Harding wrote:
> > Currently script just dumps all results found. Potentially, this risks
> > loosing single results among multiple duplicate results. We need some
> > way of restricting duplicates to assist users of the script. It would
> > also be nice if we got a report instead of raw results.
> > 
> > Duplicates can be defined in various ways, instead of trying to find a
> > single perfect solution we can present the user with various options to
> > display the output. Doing so will typically lead to users wanting to
> > view the output multiple times. Currently we scan the kernel each time,
> > this is slow and unnecessary. We can expedite the process by writing the
> > results to file for subsequent viewing.
> > 
> > Add sub-commands `scan` and `format`. Display output as a report instead
> > of raw results. Add --raw flag to view raw results. Save results to
> > file. For subsequent calls to `format` parse output file instead of
> > re-scanning.
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  scripts/leaking_addresses.pl | 201 ++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 187 insertions(+), 14 deletions(-)
> > 
> > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> > index 719ed0aaede7..4c31e935319b 100755
> > --- a/scripts/leaking_addresses.pl
> > +++ b/scripts/leaking_addresses.pl
> > @@ -21,14 +21,19 @@ use File::Spec;
> >  use Cwd 'abs_path';
> >  use Term::ANSIColor qw(:constants);
> >  use Getopt::Long qw(:config no_auto_abbrev);
> > +use File::Spec::Functions 'catfile';
> >  
> >  my $P = $0;
> >  my $V = '0.01';
> >  
> > -# Directories to scan.
> > +# Directories to scan (we scan `dmesg` also).
> >  my @DIRS = ('/proc', '/sys');
> >  
> >  # Command line options.
> > +my $output = "scan.out";
> 
> The hard-coded filename and its use is dangerous. Nobody expects that
> this kind of script writes/re-writes a file on the system.

Understood.

> > +my $suppress_dmesg = 0;
> > +my $squash_by_path = 0;
> > +my $raw = 0;
> >  my $help = 0;
> >  my $debug = 0;
> >  
> > @@ -70,21 +75,34 @@ sub help
> >  	my ($exitcode) = @_;
> >  
> >  	print << "EOM";
> > -Usage: $P [OPTIONS]
> > +
> > +Usage: $P COMMAND [OPTIONS]
> >  Version: $V
> >  
> > +Commands:
> > +
> > +	scan	Scan the kernel (savesg raw results to file and runs `format`).
> > +	format	Parse results file and format output.
> 
> The later formatting/filtering might be useful but the use
> of the hard coded file is strange. Also it is pity that
> the script does not do anything useful out of box.
> 
> I suggest to remove the commands and do the scan out of box.
> It should not store anything on the disk by default.
> 
> Then we could define following options:
> 
>     -o, --output=<file>	 Store raw results into file for later formatting.
>     -i, --input=<file>   Read raw result from file and just format them.
> 
> Well, it is still somehow non-intuitive. It might help to
> be more explicit:
> 
>     -o, --output-raw=<file>
>     -i, --input-raw=<file>

So,

 Usage: scripts/leaking_addresses.pl [OPTIONS]

 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.

> >  Options:
> >  
> > -	-d, --debug                Display debugging output.
> > -	-h, --help, --version      Display this help and exit.
> > +	-o, --output=<file>	 Raw results output file, used for later formatting.
> > +	    --suppress-dmesg	 Do not show dmesg results.
> > +	    --squash-by-path	 Show one result per unique path.
> 
> I would personally add also option for the default mode:
> 
> 	    --squash-by-filename Show one result per unique filename
> 				 (default).
> 
> In fact, I would personally use --squash-by-path or even --raw by
> default. These provide easy to understand output. While the
> --squash-by-filename mode has pretty good results but
> it is quite non-intuitive.

Thanks for you suggestions Petr. Summary reporting by default was
suggested by Linus, but now the summary is implemented and has proven to
be heuristic I tend to agree with you that raw by default is
best. This gives users the information they need to select one of the
summary types i.e if raw output has a bunch of lines from different
paths but all filename FOO then --squash-by-filename may be used.

Thanks for the tips, it's much nicer without the sub-commands.

thanks,
Tobin.

[PATCH 5/7] scripts/leaking_addresses: add emailing results

[Tobin C. Harding]

Developers may not have the time (or inclination) to investigate script
output. This information is, however, useful. If we add functionality to
the script to email results for further investigation.

Add --send-report flag to email scan results (to Tobin C. Harding).

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 mode change 100755 => 100644 scripts/leaking_addresses.pl

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
old mode 100755
new mode 100644
index 4c31e935319b..e43105662306
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -34,6 +34,7 @@ my $output = "scan.out";
 my $suppress_dmesg = 0;
 my $squash_by_path = 0;
 my $raw = 0;
+my $send_report = 0;
 my $help = 0;
 my $debug = 0;
 
@@ -90,6 +91,7 @@ Options:
 	    --suppress-dmesg	 Do not show dmesg results.
 	    --squash-by-path	 Show one result per unique path.
 	    --raw	 	 Show raw results.
+	    --send-report	 Submit raw results for someone else to worry about.
 	-d, --debug              Display debugging output.
 	-h, --help, --version    Display this help and exit.
 
@@ -103,6 +105,7 @@ GetOptions(
 	'suppress-dmesg'	=> \$suppress_dmesg,
 	'squash-by-path'	=> \$squash_by_path,
 	'raw'			=> \$raw,
+	'send-report'		=> \$send_report,
 	'd|debug'		=> \$debug,
 	'h|help'		=> \$help,
 	'version'		=> \$help
@@ -124,6 +127,12 @@ if ($command eq 'scan') {
 	scan();
 }
 
+if ($send_report) {
+	send_report();
+	print "Raw scan results sent, thank you.\n";
+	exit(0);
+}
+
 format_output();
 
 exit 0;
@@ -144,6 +153,39 @@ sub scan
 	select STDOUT;
 }
 
+sub send_report
+{
+	my $subject = 'LEAK REPORT';
+	my $email = 'leaks@tobin.cc';
+
+	my $message = sprintf("kptr_restrict: %s\n", get_kptr_restrict());
+
+	# Slurp raw results.
+	$message .= do {
+		local $/ = undef;
+		open my $fh, "<", $output
+		    or die "could not open $output: $!";
+		<$fh>;
+	};
+
+	open my $mailh, '|-', "mail -s '$subject' $email"
+	    or die( "Could not open pipe! $!" );
+
+	print $mailh $message;
+	close $mailh;
+}
+
+sub get_kptr_restrict
+{
+	my $filename = "/proc/sys/kernel/kptr_restrict";
+	my @array = do {
+		open my $fh, "<", $filename
+		    or die "could not open $filename: $!";
+		<$fh>;
+	};
+	return $array[0];
+}
+
 sub is_false_positive
 {
 	my ($match) = @_;
-- 
2.7.4

Re: [PATCH 5/7] scripts/leaking_addresses: add emailing results

[Petr Mladek]

On Wed 2017-11-08 14:37:37, Tobin C. Harding wrote:
> Developers may not have the time (or inclination) to investigate script
> output. This information is, however, useful. If we add functionality to
> the script to email results for further investigation.
> 
> Add --send-report flag to email scan results (to Tobin C. Harding).

I am not sure that it is wise to make spaming one person
so easy ;-)

It might make sense to add some more information into
the message. For example:

    + uname -a
    + whether the log was generated using root access

Also people might feel more comfortable if this feature:

     + prints the message
     + printks where it is being sent
     + ask yes/no before doing so


>  scripts/leaking_addresses.pl | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  mode change 100755 => 100644 scripts/leaking_addresses.pl
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> old mode 100755
> new mode 100644

I guess that this was not intended.

Best Regards,
Petr

Re: [PATCH 5/7] scripts/leaking_addresses: add emailing results

[Greg KH]

On Wed, Nov 08, 2017 at 11:16:43AM +0100, Petr Mladek wrote:
> On Wed 2017-11-08 14:37:37, Tobin C. Harding wrote:
> > Developers may not have the time (or inclination) to investigate script
> > output. This information is, however, useful. If we add functionality to
> > the script to email results for further investigation.
> > 
> > Add --send-report flag to email scan results (to Tobin C. Harding).
> 
> I am not sure that it is wise to make spaming one person
> so easy ;-)

I agree, I would strongly discourage this, as you will end up getting
reports from really old kernels for the next 20+ years.  We have seen
that happen for every time we have added a "report this to foo@baz" in a
kernel log message.

If you _really_ want to do this, at least point it at a mailing list.

thanks,

greg k-h

Re: [PATCH 5/7] scripts/leaking_addresses: add emailing results

[Tobin C. Harding]

On Wed, Nov 08, 2017 at 12:51:20PM +0100, Greg KH wrote:
> On Wed, Nov 08, 2017 at 11:16:43AM +0100, Petr Mladek wrote:
> > On Wed 2017-11-08 14:37:37, Tobin C. Harding wrote:
> > > Developers may not have the time (or inclination) to investigate script
> > > output. This information is, however, useful. If we add functionality to
> > > the script to email results for further investigation.
> > > 
> > > Add --send-report flag to email scan results (to Tobin C. Harding).
> > 
> > I am not sure that it is wise to make spaming one person
> > so easy ;-)
> 
> I agree, I would strongly discourage this, as you will end up getting
> reports from really old kernels for the next 20+ years.  We have seen
> that happen for every time we have added a "report this to foo@baz" in a
> kernel log message.
> 
> If you _really_ want to do this, at least point it at a mailing list.

Will remove --send-report for next version.

thanks,
Tobin.

[PATCH 6/7] scripts/leaking_addresses: fix comment typo

[Tobin C. Harding]

Fix typo in comment string.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index e43105662306..0671aac894be 100644
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -46,7 +46,7 @@ my @skip_parse_files_abs = ('/proc/kmsg',
 			    '/sys/kernel/debug/tracing/trace_pipe',
 			    '/sys/kernel/security/apparmor/revision');
 
-# Do not parse thes files under any subdirectory.
+# Do not parse these files under any subdirectory.
 my @skip_parse_files_any = ('0',
 			    '1',
 			    '2',
-- 
2.7.4

[PATCH 7/7] scripts/leaking_addresses: don't parse usbmon

[Tobin C. Harding]

file 'usbmon' causes script to hang.

Add usbmon to files to skip under any sub directory.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 scripts/leaking_addresses.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 0671aac894be..501a4759137f 100644
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -69,7 +69,8 @@ my @skip_walk_dirs_any = ('self',
 			  'fd',
 			  'stderr',
 			  'stdin',
-			  'stdout');
+			  'stdout',
+			  'usbmon');
 
 sub help
 {
-- 
2.7.4