[GIT PULL] leaking_addresses.pl changes for 4.16-rc1

[GIT PULL] leaking_addresses.pl changes for 4.16-rc1

[Tobin C. Harding]

The following changes since commit d8a5b80568a9cb66810e75b182018e9edb68e8ff:

  Linux 4.15 (2018-01-28 13:20:33 -0800)

are available in the git repository at:

  git://github.com/tcharding/linux.git tags/leaks-4.16-rc1

for you to fetch changes up to 46753437945535271a557dbf9dcb3ea53f1755e5:

  leaking_addresses: add 32-bit support (2018-01-31 08:18:51 +1100)

----------------------------------------------------------------
leaking_addresses patches for 4.16-rc1

Here is the patch set for changes to scripts/leaking_addresses.pl for
the 4.16-rc1 merge window.  The first 4 patches are clean up.  Then we
add the following functionality:

  - check addresses against the vsyscall memory range instead of just
    the first and last address (x86_64)
  - add support for getting config options from the kernel config file
  - add support for 5 page table levels (x86_64)
  - add support for scanning 32 bit kernels (based on the page offset)

Along the way we add some helper sub routines and use `uname -m` instead
of Perl for doing architecture detection.  All these patches, except the
trivial clean up ones, were posted to LKML.  The script with this set
applied has been tested on x86_64 (kernel 4.4 and 4.15), ppc64 (kernel
4.4) and ARM 32-bit (kernel 4.9).

Signed-off-by: Tobin C. Harding <me@tobin.cc>

----------------------------------------------------------------
Tobin C. Harding (10):
      leaking_addresses: fix typo function not called
      leaking_addresses: remove mention of kptr_restrict
      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
      leaking_addresses: use system command to get arch
      leaking_addresses: add is_arch() wrapper subroutine
      leaking_addresses: add 32-bit support

 scripts/leaking_addresses.pl | 261 +++++++++++++++++++++++++++++++++----------
 1 file changed, 205 insertions(+), 56 deletions(-)

Re: [GIT PULL] leaking_addresses.pl changes for 4.16-rc1

[Tobin C. Harding]

On Wed, Jan 31, 2018 at 01:42:36PM +1100, Tobin C. Harding wrote:
> The following changes since commit d8a5b80568a9cb66810e75b182018e9edb68e8ff:
> 
>   Linux 4.15 (2018-01-28 13:20:33 -0800)
> 
> are available in the git repository at:
> 
>   git://github.com/tcharding/linux.git tags/leaks-4.16-rc1
> 
> for you to fetch changes up to 46753437945535271a557dbf9dcb3ea53f1755e5:
> 
>   leaking_addresses: add 32-bit support (2018-01-31 08:18:51 +1100)

Hi Linus,

It has just come to my attention that I should have pushed these changes
to Linux next before requesting you to pull them.  Please feel free to
drop this request, I can try again next merge window after going through
linux next.

thanks,
Tobin.

Re: [GIT PULL] leaking_addresses.pl changes for 4.16-rc1

[Linus Torvalds]

On Thu, Feb 1, 2018 at 12:45 PM, Tobin C. Harding <me@tobin.cc> wrote:
>
> It has just come to my attention that I should have pushed these changes
> to Linux next before requesting you to pull them.  Please feel free to
> drop this request, I can try again next merge window after going through
> linux next.

For something like this, I don't think it's a big deal.

A bigger deal is that it now wants perl-bigint, as of commit
8d8a77fb99bd ("leaking_addresses: add range check for vsyscall
memory").

And that is not apparently a common enough perl module to be installed
by default.

Sure, I just ran

    dnf install 'perl(bigint)'

and it did the right thing, but it does seem to be something of an
inconvenience.

And things are *slow*, to the point of breakage. I get

  timed out parsing: /proc/kallsyms
  timed out parsing: /proc/2177/smaps
  timed out parsing: /proc/2238/smaps
  timed out parsing: /proc/2336/smaps
  ...

and no actual output. I'm not sure what's up with that, and whether
it's related. Probably not, but I didn't start bisecting.

So I pulled it but then unpulled it due to issues during testing.

             Linus

Re: [GIT PULL] leaking_addresses.pl changes for 4.16-rc1

[Tobin C. Harding]

On Thu, Feb 01, 2018 at 02:43:28PM -0800, Linus Torvalds wrote:
> On Thu, Feb 1, 2018 at 12:45 PM, Tobin C. Harding <me@tobin.cc> wrote:
> >
> > It has just come to my attention that I should have pushed these changes
> > to Linux next before requesting you to pull them.  Please feel free to
> > drop this request, I can try again next merge window after going through
> > linux next.
> 
> For something like this, I don't think it's a big deal.
> 
> A bigger deal is that it now wants perl-bigint, as of commit
> 8d8a77fb99bd ("leaking_addresses: add range check for vsyscall
> memory").
> 
> And that is not apparently a common enough perl module to be installed
> by default.
> 
> Sure, I just ran
> 
>     dnf install 'perl(bigint)'
> 
> and it did the right thing, but it does seem to be something of an
> inconvenience.

ok I'll try and find a work around so as not to use that.

> And things are *slow*, to the point of breakage. I get
> 
>   timed out parsing: /proc/kallsyms
>   timed out parsing: /proc/2177/smaps
>   timed out parsing: /proc/2238/smaps
>   timed out parsing: /proc/2336/smaps
>   ...

TL;DR it's working just not behaving very well.

I knew that was an issue, I've been running tests with 'smaps' included
in the skip files array.  However the patch I submitted to add this
included skipping /proc/kallsysms.  This got nacked.  I could not come
up with a _good_ solution before doing the pull request and thought it
better to have the 32-bit stuff in in light of all the drama this last
month.  The problem is that timeouts were added to catch binary files
choking the script but the default is too slow for large ASCII files.
Also we don't have a way to say 'scan this file once but not in every
processes sub directory' i.e smaps.  I needed feedback on this since I'm
not totally sure that all smaps files are generated the same.  Also I
think there may be a better way to not pass binary files than using the
timeout.

> and no actual output. I'm not sure what's up with that, and whether
> it's related. Probably not, but I didn't start bisecting.

No output is good - it means no leaks, though I'm still getting a bunch
from dmesg on the 3 machines I tested on.

> So I pulled it but then unpulled it due to issues during testing.

If it is not super important to have 32-bit scanning available then I'd
just as well see this wait until next merge window and getting it
running better.  In hindsight all of this should probably been in the
pull request message.

thanks for taking the time on this.

	Tobin

Re: [GIT PULL] leaking_addresses.pl changes for 4.16-rc1

[Tobin C. Harding]

On Fri, Feb 02, 2018 at 08:48:10PM +1100, Tobin C. Harding wrote:
> On Thu, Feb 01, 2018 at 02:43:28PM -0800, Linus Torvalds wrote:
> > On Thu, Feb 1, 2018 at 12:45 PM, Tobin C. Harding <me@tobin.cc> wrote:
> > >
> > > It has just come to my attention that I should have pushed these changes
> > > to Linux next before requesting you to pull them.  Please feel free to
> > > drop this request, I can try again next merge window after going through
> > > linux next.
> > 
> > For something like this, I don't think it's a big deal.
> > 
> > A bigger deal is that it now wants perl-bigint, as of commit
> > 8d8a77fb99bd ("leaking_addresses: add range check for vsyscall
> > memory").
> > 
> > And that is not apparently a common enough perl module to be installed
> > by default.
> > 
> > Sure, I just ran
> > 
> >     dnf install 'perl(bigint)'
> > 
> > and it did the right thing, but it does seem to be something of an
> > inconvenience.
> 
> ok I'll try and find a work around so as not to use that.
> 
> > And things are *slow*, to the point of breakage. I get
> > 
> >   timed out parsing: /proc/kallsyms
> >   timed out parsing: /proc/2177/smaps
> >   timed out parsing: /proc/2238/smaps
> >   timed out parsing: /proc/2336/smaps
> >   ...
> 
> TL;DR it's working just not behaving very well.
> 
> I knew that was an issue, I've been running tests with 'smaps' included
> in the skip files array.  However the patch I submitted to add this
> included skipping /proc/kallsysms.  This got nacked.  I could not come
> up with a _good_ solution before doing the pull request and thought it
> better to have the 32-bit stuff in in light of all the drama this last
> month.  The problem is that timeouts were added to catch binary files
> choking the script but the default is too slow for large ASCII files.
> Also we don't have a way to say 'scan this file once but not in every
> processes sub directory' i.e smaps.  I needed feedback on this since I'm
> not totally sure that all smaps files are generated the same.  Also I
> think there may be a better way to not pass binary files than using the
> timeout.

oops, the timeout was to catch files that block on read (like /proc/kmsg).

So, adding 'smaps' to @skip_parse_files_any makes it run but I am unsure
if this is correct.  I am under the impression that there will be
various files that need skipping once the script is run on other peoples
setups.


thanks,
Tobin.