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
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 <[email protected]>
Signed-off-by: Tobin C. Harding <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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
On Tue, Feb 27, 2018 at 6:45 AM, Tobin C. Harding <[email protected]> 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 <[email protected]>
> ---
> 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
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 <[email protected]> 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 <[email protected]>
> > ---
> > 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.
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 <[email protected]>
> ---
> 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.
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 <[email protected]>
> > ---
> > 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
On Thu, Mar 1, 2018 at 11:06 PM, Tobin C. Harding <[email protected]> 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 <[email protected]>
>> ---
>> 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.