linux-kernel - Re: [PATCH 4/7] scripts/leaking

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]
Message-ID: <20171108104221.GH2652@pathway.suse.cz>
Date:   Wed, 8 Nov 2017 11:42:21 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     "Tobin C. Harding" <me@...in.cc>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Theodore Ts'o <tytso@....edu>,
        Kees Cook <keescook@...omium.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Tycho Andersen <tycho@...ker.com>,
        "Roberts, William C" <william.c.roberts@...el.com>,
        Tejun Heo <tj@...nel.org>,
        Jordan Glover <Golden_Miller83@...tonmail.ch>,
        Greg KH <gregkh@...uxfoundation.org>,
        Joe Perches <joe@...ches.com>,
        Ian Campbell <ijc@...lion.org.uk>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <wilal.deacon@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Chris Fries <cfries@...gle.com>,
        Dave Weinstein <olorin@...gle.com>,
        Daniel Micay <danielmicay@...il.com>,
        Djalal Harouni <tixxdz@...il.com>,
        linux-kernel@...r.kernel.org,
        Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        kernel-hardening@...ts.openwall.com,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 4/7] scripts/leaking_addresses: add reporting

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@...in.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