[PATCH 0/3] leaking_addresses: limit scan to PID==1

[PATCH 0/3] leaking_addresses: limit scan to PID==1

[Tobin C. Harding]

This set implements improvements discussed offline with Tycho as well as
from suggestions on LKML.

We no longer bother to scan /proc/PID for every PID on the system.
Instead we only scan /proc/1 (still scan other non-pid related
files/directoies).  The reasoning is given in the commit log of patch 1,
duplicated here for reference:

	When the system is idle it is likely that most files under
	/proc/PID will be identical for various processes.  Scanning
	_all_ the PIDs under /proc is unnecessary and implies that we
	are thoroughly scanning /proc. This is _not_ the case because
	there may be ways userspace can trigger creation of /proc files
	that leak addresses but were not present during a scan.  For
	these two reasons we should exclude all PID directories under
	/proc except '1/' 
    
Next, we skip parsing /proc/1/syscall as suggested because the pointers
listed are user pointers, and negative syscall args will show up like
kernel pointers.

Finally we remove version number from the script.

This set represents the tip of the branch 'leaks-testing' available at

	git://git.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git


thanks,
Tobin.


Tobin C. Harding (3):
  leaking_addresses: skip all /proc/PID except /proc/1
  leaking_addresses: skip '/proc/1/syscall'
  leaking_addresses: remove version number

 scripts/leaking_addresses.pl | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Tobin C. Harding]

When the system is idle it is likely that most files under /proc/PID
will be identical for various processes.  Scanning _all_ the PIDs under
/proc is unnecessary and implies that we are thoroughly scanning /proc.
This is _not_ the case because there may be ways userspace can trigger
creation of /proc files that leak addresses but were not present during
a scan.  For these two reasons we should exclude all PID directories
under /proc except '1/'

Exclude all /proc/PID except /proc/1.

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

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 6e5bc57caeaa..fb40e2828f43 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -10,6 +10,14 @@
 # Use --debug to output path before parsing, this is useful to find files that
 # cause the script to choke.
 
+#
+# When the system is idle it is likely that most files under /proc/PID will be
+# identical for various processes.  Scanning _all_ the PIDs under /proc is
+# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
+# the case because there may be ways userspace can trigger creation of /proc
+# files that leak addresses but were not present during a scan.  For these two
+# reasons we exclude all PID directories under /proc except '1/'
+
 use warnings;
 use strict;
 use POSIX;
@@ -472,6 +480,9 @@ sub walk
 			my $path = "$pwd/$file";
 			next if (-l $path);
 
+			# skip /proc/PID except /proc/1
+			next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);
+
 			next if (skip($path));
 
 			if (-d $path) {
-- 
2.7.4

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Tycho Andersen]

Hi Tobin,

On Tue, Feb 27, 2018 at 03:45:09PM +1100, Tobin C. Harding wrote:
> When the system is idle it is likely that most files under /proc/PID
> will be identical for various processes.  Scanning _all_ the PIDs under
> /proc is unnecessary and implies that we are thoroughly scanning /proc.
> This is _not_ the case because there may be ways userspace can trigger
> creation of /proc files that leak addresses but were not present during
> a scan.  For these two reasons we should exclude all PID directories
> under /proc except '1/'
> 
> Exclude all /proc/PID except /proc/1.
> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  scripts/leaking_addresses.pl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 6e5bc57caeaa..fb40e2828f43 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -10,6 +10,14 @@
>  # Use --debug to output path before parsing, this is useful to find files that
>  # cause the script to choke.
>  
> +#
> +# When the system is idle it is likely that most files under /proc/PID will be
> +# identical for various processes.  Scanning _all_ the PIDs under /proc is
> +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
> +# the case because there may be ways userspace can trigger creation of /proc
> +# files that leak addresses but were not present during a scan.  For these two
> +# reasons we exclude all PID directories under /proc except '1/'
> +
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -472,6 +480,9 @@ sub walk
>  			my $path = "$pwd/$file";
>  			next if (-l $path);
>  
> +			# skip /proc/PID except /proc/1
> +			next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);

Can't we just do,

substr($path, 0, len("/proc/1/")) eq "/proc/1/" ?

seems much easier to read than the regex.

Cheers,

Tycho

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Tobin C. Harding]

On Mon, Feb 26, 2018 at 10:09:31PM -0700, Tycho Andersen wrote:
> Hi Tobin,
> 
> On Tue, Feb 27, 2018 at 03:45:09PM +1100, Tobin C. Harding wrote:
> > When the system is idle it is likely that most files under /proc/PID
> > will be identical for various processes.  Scanning _all_ the PIDs under
> > /proc is unnecessary and implies that we are thoroughly scanning /proc.
> > This is _not_ the case because there may be ways userspace can trigger
> > creation of /proc files that leak addresses but were not present during
> > a scan.  For these two reasons we should exclude all PID directories
> > under /proc except '1/'
> > 
> > Exclude all /proc/PID except /proc/1.
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  scripts/leaking_addresses.pl | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> > index 6e5bc57caeaa..fb40e2828f43 100755
> > --- a/scripts/leaking_addresses.pl
> > +++ b/scripts/leaking_addresses.pl
> > @@ -10,6 +10,14 @@
> >  # Use --debug to output path before parsing, this is useful to find files that
> >  # cause the script to choke.
> >  
> > +#
> > +# When the system is idle it is likely that most files under /proc/PID will be
> > +# identical for various processes.  Scanning _all_ the PIDs under /proc is
> > +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
> > +# the case because there may be ways userspace can trigger creation of /proc
> > +# files that leak addresses but were not present during a scan.  For these two
> > +# reasons we exclude all PID directories under /proc except '1/'
> > +
> >  use warnings;
> >  use strict;
> >  use POSIX;
> > @@ -472,6 +480,9 @@ sub walk
> >  			my $path = "$pwd/$file";
> >  			next if (-l $path);
> >  
> > +			# skip /proc/PID except /proc/1
> > +			next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);
> 
> Can't we just do,
> 
> substr($path, 0, len("/proc/1/")) eq "/proc/1/" ?
> 
> seems much easier to read than the regex.

This is much better.  I guess it's true what they say, be careful after
reading a book about hammers, everything will look like a nail.


	Tobin

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Alexander Kapshuk]

On Tue, Feb 27, 2018 at 6:45 AM, Tobin C. Harding <me@tobin.cc> wrote:
> When the system is idle it is likely that most files under /proc/PID
> will be identical for various processes.  Scanning _all_ the PIDs under
> /proc is unnecessary and implies that we are thoroughly scanning /proc.
> This is _not_ the case because there may be ways userspace can trigger
> creation of /proc files that leak addresses but were not present during
> a scan.  For these two reasons we should exclude all PID directories
> under /proc except '1/'
>
> Exclude all /proc/PID except /proc/1.
>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  scripts/leaking_addresses.pl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 6e5bc57caeaa..fb40e2828f43 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -10,6 +10,14 @@
>  # Use --debug to output path before parsing, this is useful to find files that
>  # cause the script to choke.
>
> +#
> +# When the system is idle it is likely that most files under /proc/PID will be
> +# identical for various processes.  Scanning _all_ the PIDs under /proc is
> +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
> +# the case because there may be ways userspace can trigger creation of /proc
> +# files that leak addresses but were not present during a scan.  For these two
> +# reasons we exclude all PID directories under /proc except '1/'
> +
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -472,6 +480,9 @@ sub walk
>                         my $path = "$pwd/$file";
>                         next if (-l $path);
>
> +                       # skip /proc/PID except /proc/1
> +                       next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);
> +
>                         next if (skip($path));
>
>                         if (-d $path) {
> --
> 2.7.4
>

Would something like this do the trick?
perl -e 'foreach my $dir (`ls -d /proc/[0-9]*`){next if($dir !~
"/proc/1\$"); print $dir}'
/proc/1

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Tobin C. Harding]

On Tue, Feb 27, 2018 at 09:15:03AM +0200, Alexander Kapshuk wrote:
> On Tue, Feb 27, 2018 at 6:45 AM, Tobin C. Harding <me@tobin.cc> wrote:
> > When the system is idle it is likely that most files under /proc/PID
> > will be identical for various processes.  Scanning _all_ the PIDs under
> > /proc is unnecessary and implies that we are thoroughly scanning /proc.
> > This is _not_ the case because there may be ways userspace can trigger
> > creation of /proc files that leak addresses but were not present during
> > a scan.  For these two reasons we should exclude all PID directories
> > under /proc except '1/'
> >
> > Exclude all /proc/PID except /proc/1.
> >
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  scripts/leaking_addresses.pl | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> > index 6e5bc57caeaa..fb40e2828f43 100755
> > --- a/scripts/leaking_addresses.pl
> > +++ b/scripts/leaking_addresses.pl
> > @@ -10,6 +10,14 @@
> >  # Use --debug to output path before parsing, this is useful to find files that
> >  # cause the script to choke.
> >
> > +#
> > +# When the system is idle it is likely that most files under /proc/PID will be
> > +# identical for various processes.  Scanning _all_ the PIDs under /proc is
> > +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
> > +# the case because there may be ways userspace can trigger creation of /proc
> > +# files that leak addresses but were not present during a scan.  For these two
> > +# reasons we exclude all PID directories under /proc except '1/'
> > +
> >  use warnings;
> >  use strict;
> >  use POSIX;
> > @@ -472,6 +480,9 @@ sub walk
> >                         my $path = "$pwd/$file";
> >                         next if (-l $path);
> >
> > +                       # skip /proc/PID except /proc/1
> > +                       next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);
> > +
> >                         next if (skip($path));
> >
> >                         if (-d $path) {
> > --
> > 2.7.4
> >
> 
> Would something like this do the trick?
> perl -e 'foreach my $dir (`ls -d /proc/[0-9]*`){next if($dir !~
> "/proc/1\$"); print $dir}'
> /proc/1

thanks for the suggestion Alexander.  Here is Tycho's suggestion (from
other email, copied here for reference:

	> substr($path, 0, len("/proc/1/")) eq "/proc/1/"

I originally thought Tycho's suggestion was correct until I read yours
and realized that they both find '/proc/1'.  You filter on the numbered
directories for '/proc/1' (missing the other directories) and Tycho
finds only directories with '/proc/1' as the leading characters.  Both
of these differ to the original regex in that the original skips
numbered directories (under '/proc') that are _not_ '/proc/1' i.e it
allows parsing of all the non-numbered directories and parsing of '/proc/1'.

If my reasoning is correct, perhaps we have at least shown that that the
regex should have a comment :)

Happy to be corrected.

thanks,
Tobin.

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Tobin C. Harding]

On Tue, Feb 27, 2018 at 03:45:09PM +1100, Tobin C. Harding wrote:
> When the system is idle it is likely that most files under /proc/PID
> will be identical for various processes.  Scanning _all_ the PIDs under
> /proc is unnecessary and implies that we are thoroughly scanning /proc.
> This is _not_ the case because there may be ways userspace can trigger
> creation of /proc files that leak addresses but were not present during
> a scan.  For these two reasons we should exclude all PID directories
> under /proc except '1/'
> 
> Exclude all /proc/PID except /proc/1.
> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  scripts/leaking_addresses.pl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 6e5bc57caeaa..fb40e2828f43 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -10,6 +10,14 @@
>  # Use --debug to output path before parsing, this is useful to find files that
>  # cause the script to choke.
>  
> +#
> +# When the system is idle it is likely that most files under /proc/PID will be
> +# identical for various processes.  Scanning _all_ the PIDs under /proc is
> +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
> +# the case because there may be ways userspace can trigger creation of /proc
> +# files that leak addresses but were not present during a scan.  For these two
> +# reasons we exclude all PID directories under /proc except '1/'
> +
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -472,6 +480,9 @@ sub walk
>  			my $path = "$pwd/$file";
>  			next if (-l $path);
>  
> +			# skip /proc/PID except /proc/1
> +			next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);

Perhaps the intent of this is clearer?

			next if (($path =~ /^\/proc\/[0-9]+$/) &&
			     	 ($path !~ /^\/proc\/1$/));


thanks,
Tobin.

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Tycho Andersen]

On Fri, Mar 02, 2018 at 08:06:23AM +1100, Tobin C. Harding wrote:
> On Tue, Feb 27, 2018 at 03:45:09PM +1100, Tobin C. Harding wrote:
> > When the system is idle it is likely that most files under /proc/PID
> > will be identical for various processes.  Scanning _all_ the PIDs under
> > /proc is unnecessary and implies that we are thoroughly scanning /proc.
> > This is _not_ the case because there may be ways userspace can trigger
> > creation of /proc files that leak addresses but were not present during
> > a scan.  For these two reasons we should exclude all PID directories
> > under /proc except '1/'
> > 
> > Exclude all /proc/PID except /proc/1.
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  scripts/leaking_addresses.pl | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> > index 6e5bc57caeaa..fb40e2828f43 100755
> > --- a/scripts/leaking_addresses.pl
> > +++ b/scripts/leaking_addresses.pl
> > @@ -10,6 +10,14 @@
> >  # Use --debug to output path before parsing, this is useful to find files that
> >  # cause the script to choke.
> >  
> > +#
> > +# When the system is idle it is likely that most files under /proc/PID will be
> > +# identical for various processes.  Scanning _all_ the PIDs under /proc is
> > +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
> > +# the case because there may be ways userspace can trigger creation of /proc
> > +# files that leak addresses but were not present during a scan.  For these two
> > +# reasons we exclude all PID directories under /proc except '1/'
> > +
> >  use warnings;
> >  use strict;
> >  use POSIX;
> > @@ -472,6 +480,9 @@ sub walk
> >  			my $path = "$pwd/$file";
> >  			next if (-l $path);
> >  
> > +			# skip /proc/PID except /proc/1
> > +			next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);
> 
> Perhaps the intent of this is clearer?
> 
> 			next if (($path =~ /^\/proc\/[0-9]+$/) &&
> 			     	 ($path !~ /^\/proc\/1$/));

+1, works for me.

Cheers,

Tycho

Re: [PATCH 1/3] leaking_addresses: skip all /proc/PID except /proc/1

[Alexander Kapshuk]

On Thu, Mar 1, 2018 at 11:06 PM, Tobin C. Harding <me@tobin.cc> wrote:
> On Tue, Feb 27, 2018 at 03:45:09PM +1100, Tobin C. Harding wrote:
>> When the system is idle it is likely that most files under /proc/PID
>> will be identical for various processes.  Scanning _all_ the PIDs under
>> /proc is unnecessary and implies that we are thoroughly scanning /proc.
>> This is _not_ the case because there may be ways userspace can trigger
>> creation of /proc files that leak addresses but were not present during
>> a scan.  For these two reasons we should exclude all PID directories
>> under /proc except '1/'
>>
>> Exclude all /proc/PID except /proc/1.
>>
>> Signed-off-by: Tobin C. Harding <me@tobin.cc>
>> ---
>>  scripts/leaking_addresses.pl | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
>> index 6e5bc57caeaa..fb40e2828f43 100755
>> --- a/scripts/leaking_addresses.pl
>> +++ b/scripts/leaking_addresses.pl
>> @@ -10,6 +10,14 @@
>>  # Use --debug to output path before parsing, this is useful to find files that
>>  # cause the script to choke.
>>
>> +#
>> +# When the system is idle it is likely that most files under /proc/PID will be
>> +# identical for various processes.  Scanning _all_ the PIDs under /proc is
>> +# unnecessary and implies that we are thoroughly scanning /proc.  This is _not_
>> +# the case because there may be ways userspace can trigger creation of /proc
>> +# files that leak addresses but were not present during a scan.  For these two
>> +# reasons we exclude all PID directories under /proc except '1/'
>> +
>>  use warnings;
>>  use strict;
>>  use POSIX;
>> @@ -472,6 +480,9 @@ sub walk
>>                       my $path = "$pwd/$file";
>>                       next if (-l $path);
>>
>> +                     # skip /proc/PID except /proc/1
>> +                     next if ($path =~ /\/proc\/(?:[2-9][0-9]*|1[0-9]+)/);
>
> Perhaps the intent of this is clearer?
>
>                         next if (($path =~ /^\/proc\/[0-9]+$/) &&
>                                  ($path !~ /^\/proc\/1$/));
>
>
> thanks,
> Tobin.

Hi Tobin,

The intent is crystal clear now. Thanks.

Here's something that generates the same output as the code above:
next if ($path !~ "^/proc/(1|[^0-9]+)\$");

I'm not insisting this be given any preference whatsoever.

Thanks.

[PATCH 2/3] leaking_addresses: skip '/proc/1/syscall'

[Tobin C. Harding]

The pointers listed in /proc/1/syscall are user pointers, and negative
syscall args will show up like kernel addresses.

For example

/proc/31808/syscall: 0 0x3 0x55b107a38180 0x2000 0xffffffffffffffb0 \
0x55b107a302d0 0x55b107a38180 0x7fffa313b8e8 0x7ff098560d11

Skip parsing /proc/1/syscall

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

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index fb40e2828f43..ac84a164a528 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -60,6 +60,7 @@ my $page_offset_32bit = 0;	# Page offset for 32-bit kernel.
 my @skip_abs = (
 	'/proc/kmsg',
 	'/proc/device-tree',
+	'/proc/1/syscall',
 	'/sys/firmware/devicetree',
 	'/sys/kernel/debug/tracing/trace_pipe',
 	'/sys/kernel/security/apparmor/revision');
-- 
2.7.4

[PATCH 3/3] leaking_addresses: remove version number

[Tobin C. Harding]

We have git now, we don't need a version number.  This was originally
added because leaking_addresses.pl shamelessly (and mindlessly) copied
checkpatch.pl

Remove version number from script.

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

diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index ac84a164a528..d5d31f3df97e 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -31,7 +31,6 @@ use bigint qw/hex/;
 use feature 'state';
 
 my $P = $0;
-my $V = '0.01';
 
 # Directories to scan.
 my @DIRS = ('/proc', '/sys');
@@ -85,7 +84,6 @@ sub help
 	print << "EOM";
 
 Usage: $P [OPTIONS]
-Version: $V
 
 Options:
 
-- 
2.7.4