Re: [PATCH 1/2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Tobin C. Harding]

Hi Kaiwan,

thanks for the patches!

On Thu, Nov 23, 2017 at 10:44:00AM +0530, kaiwan.billimo...@gmail.com wrote:
> The current leaking_addresses.pl script only supports showing "leaked"
> 64-bit kernel virtual addresses. This patch adds support for showing
> "leaked" 32-bit kernel virtual addresses. It also takes into account Tobin's
> feedback on the previous iteration. (Note: this patch is meant to apply on
> the 'leaks' branch of Tobin's tree).

Please don't mention me in the commit log. Usually this sort of comment
would go below the - so it doesn't get into the kernel tree.

Perhaps

Currently leaking_addresses.pl only supports scanning 64 bit kernels. We
can scan 32 bit kernels also by ... (describe PAGE_OFFSET stuff)

> Briefly, the way it works- once it detects we're running on an i'x'86 
> platform,
> (where x=3|4|5|6), it takes this arch into account for checking. The 
> essential rationale:
>  if virt-addr >= PAGE_OFFSET => it's a kernel virtual address.
> 
> This version programatically queries and sets PAGE_OFFSET based on the
> /boot/config-$(uname -r) content. If, for any reason, this file cannot be
> used, we fallback to requesting the user to pass PAGE_OFFSET as a parameter.

This is a good start. What if we were to check a few places in order?

/boot/config
/boot/config-$(uname -r)
/proc/config.gz 

And fall back to 0xc000 with a warning printed to stderr if we can't
find it?

I'd suggest creating a sub routine get_page_offset() that returns
it. You could cache the result locally in the subroutine to make it
faster, here is a stack overflow post on how to do that


https://stackoverflow.com/questions/10841076/static-local-variables-in-perl

Or you could save it to a global and check this each time the you enter
the subroutine, which ever you fancy.

> Pending/TODO:
> - support for ARM-32

We don't need this in the git log either :)

> Feedback welcome..

Or this. Once it is in the tree no feed back will be possible.

> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 865c07649dff..0566f8055ec5 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -2,10 +2,10 @@
>  #
>  # (c) 2017 Tobin C. Harding 
>  # (c) 2017 Kaiwan N Billimoria  (ix86 support)
> - 
> +
>  # Licensed under the terms of the GNU GPL License version 2
>  #
> -# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
> +# leaking_addresses.pl: Scan 32/64 bit kernel for potential leaking 
> addresses.
>  #  - Scans dmesg output.
>  #  - Walks directory tree and parses each file (for each directory in @DIRS).
>  #
> @@ -14,7 +14,7 @@
>  #
>  # You may like to set kptr_restrict=2 before running script
>  # (see Documentation/sysctl/kernel.txt).
> -
> +#
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -37,7 +37,7 @@ my $TIMEOUT = 10;
>  # Script can only grep for kernel addresses on the following architectures. 
> If
>  # your architecture is not listed here and has a grep'able kernel address 
> please
>  # consider submitting a patch.
> -my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
> +my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
>  
>  # Command line options.
>  my $help = 0;
> @@ -49,6 +49,12 @@ my $input_raw = "";# Read raw results from file 
> instead of scanning.
>  my $suppress_dmesg = 0;  # Don't show dmesg in output.
>  my $squash_by_path = 0;  # Summary report grouped by absolute 
> path.
>  my $squash_by_filename = 0;  # Summary report grouped by filename.
> +my $page_offset_param = 0;  # 32-bit: overrides value of 
> PAGE_OFFSET_32BIT

I don't like the _param here, it's not in style with the rest of the
code. I do like the global name $PAGE_OFFSET_32BIT though. You don't
_really_ need both since the command line option _is_ a global. I also
struggled with the Perl variable nomenclature between capitals for
globals but not for command line options. (For the record I attempted to
imitate checkpatch.pl.)

 +my $page_offset = 0; # ...

> +
> +my $bit_size = 64;  # Check 64-bit kernel addresses by default

I thought we said we didn't need this?

> +my $kconfig_file = '/boot/config-'.`uname -r`;
> +$kconfig_file =~ s/\R*//g;
> +my $PAGE_OFFSET_32BIT = 0xc000;

So, the page_offset stuff still needs a bit of work. We want it as
simple as possible and also we don't want the 32 bit stuff cluttering up
the 64 bit stuff (i.e with lots of globals). For this reason I think it
would be nice to confine all this to a subroutine then we can do

if (is_ix86_32()) {
$page_offset = get_page_offset();
...
if ($addr < $page_offset)
...
}

>  # Do not parse these files (absolute path).
>  my @skip_parse_files_abs = ('/proc/kmsg',
> @@ -99,10 +105,11 @@ Options:
>  
>   -o, 

Re: [PATCH 1/2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Tobin C. Harding]

Hi Kaiwan,

thanks for the patches!

On Thu, Nov 23, 2017 at 10:44:00AM +0530, kaiwan.billimo...@gmail.com wrote:
> The current leaking_addresses.pl script only supports showing "leaked"
> 64-bit kernel virtual addresses. This patch adds support for showing
> "leaked" 32-bit kernel virtual addresses. It also takes into account Tobin's
> feedback on the previous iteration. (Note: this patch is meant to apply on
> the 'leaks' branch of Tobin's tree).

Please don't mention me in the commit log. Usually this sort of comment
would go below the - so it doesn't get into the kernel tree.

Perhaps

Currently leaking_addresses.pl only supports scanning 64 bit kernels. We
can scan 32 bit kernels also by ... (describe PAGE_OFFSET stuff)

> Briefly, the way it works- once it detects we're running on an i'x'86 
> platform,
> (where x=3|4|5|6), it takes this arch into account for checking. The 
> essential rationale:
>  if virt-addr >= PAGE_OFFSET => it's a kernel virtual address.
> 
> This version programatically queries and sets PAGE_OFFSET based on the
> /boot/config-$(uname -r) content. If, for any reason, this file cannot be
> used, we fallback to requesting the user to pass PAGE_OFFSET as a parameter.

This is a good start. What if we were to check a few places in order?

/boot/config
/boot/config-$(uname -r)
/proc/config.gz 

And fall back to 0xc000 with a warning printed to stderr if we can't
find it?

I'd suggest creating a sub routine get_page_offset() that returns
it. You could cache the result locally in the subroutine to make it
faster, here is a stack overflow post on how to do that


https://stackoverflow.com/questions/10841076/static-local-variables-in-perl

Or you could save it to a global and check this each time the you enter
the subroutine, which ever you fancy.

> Pending/TODO:
> - support for ARM-32

We don't need this in the git log either :)

> Feedback welcome..

Or this. Once it is in the tree no feed back will be possible.

> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 865c07649dff..0566f8055ec5 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -2,10 +2,10 @@
>  #
>  # (c) 2017 Tobin C. Harding 
>  # (c) 2017 Kaiwan N Billimoria  (ix86 support)
> - 
> +
>  # Licensed under the terms of the GNU GPL License version 2
>  #
> -# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
> +# leaking_addresses.pl: Scan 32/64 bit kernel for potential leaking 
> addresses.
>  #  - Scans dmesg output.
>  #  - Walks directory tree and parses each file (for each directory in @DIRS).
>  #
> @@ -14,7 +14,7 @@
>  #
>  # You may like to set kptr_restrict=2 before running script
>  # (see Documentation/sysctl/kernel.txt).
> -
> +#
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -37,7 +37,7 @@ my $TIMEOUT = 10;
>  # Script can only grep for kernel addresses on the following architectures. 
> If
>  # your architecture is not listed here and has a grep'able kernel address 
> please
>  # consider submitting a patch.
> -my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
> +my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
>  
>  # Command line options.
>  my $help = 0;
> @@ -49,6 +49,12 @@ my $input_raw = "";# Read raw results from file 
> instead of scanning.
>  my $suppress_dmesg = 0;  # Don't show dmesg in output.
>  my $squash_by_path = 0;  # Summary report grouped by absolute 
> path.
>  my $squash_by_filename = 0;  # Summary report grouped by filename.
> +my $page_offset_param = 0;  # 32-bit: overrides value of 
> PAGE_OFFSET_32BIT

I don't like the _param here, it's not in style with the rest of the
code. I do like the global name $PAGE_OFFSET_32BIT though. You don't
_really_ need both since the command line option _is_ a global. I also
struggled with the Perl variable nomenclature between capitals for
globals but not for command line options. (For the record I attempted to
imitate checkpatch.pl.)

 +my $page_offset = 0; # ...

> +
> +my $bit_size = 64;  # Check 64-bit kernel addresses by default

I thought we said we didn't need this?

> +my $kconfig_file = '/boot/config-'.`uname -r`;
> +$kconfig_file =~ s/\R*//g;
> +my $PAGE_OFFSET_32BIT = 0xc000;

So, the page_offset stuff still needs a bit of work. We want it as
simple as possible and also we don't want the 32 bit stuff cluttering up
the 64 bit stuff (i.e with lots of globals). For this reason I think it
would be nice to confine all this to a subroutine then we can do

if (is_ix86_32()) {
$page_offset = get_page_offset();
...
if ($addr < $page_offset)
...
}

>  # Do not parse these files (absolute path).
>  my @skip_parse_files_abs = ('/proc/kmsg',
> @@ -99,10 +105,11 @@ Options:
>  
>   -o, --output-raw=  Save results for future processing.
>   -i, --input-raw=   Read 

[PATCH 1/2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[kaiwan . billimoria]

The current leaking_addresses.pl script only supports showing "leaked"
64-bit kernel virtual addresses. This patch adds support for showing
"leaked" 32-bit kernel virtual addresses. It also takes into account Tobin's
feedback on the previous iteration. (Note: this patch is meant to apply on
the 'leaks' branch of Tobin's tree).

Briefly, the way it works- once it detects we're running on an i'x'86 platform,
(where x=3|4|5|6), it takes this arch into account for checking. The essential 
rationale:
 if virt-addr >= PAGE_OFFSET => it's a kernel virtual address.

This version programatically queries and sets PAGE_OFFSET based on the
/boot/config-$(uname -r) content. If, for any reason, this file cannot be
used, we fallback to requesting the user to pass PAGE_OFFSET as a parameter.

Pending/TODO:
- support for ARM-32

Feedback welcome..


Signed-off-by: Kaiwan N Billimoria 
---
diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 865c07649dff..0566f8055ec5 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -2,10 +2,10 @@
 #
 # (c) 2017 Tobin C. Harding 
 # (c) 2017 Kaiwan N Billimoria  (ix86 support)
- 
+
 # Licensed under the terms of the GNU GPL License version 2
 #
-# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
+# leaking_addresses.pl: Scan 32/64 bit kernel for potential leaking addresses.
 #  - Scans dmesg output.
 #  - Walks directory tree and parses each file (for each directory in @DIRS).
 #
@@ -14,7 +14,7 @@
 #
 # You may like to set kptr_restrict=2 before running script
 # (see Documentation/sysctl/kernel.txt).
-
+#
 use warnings;
 use strict;
 use POSIX;
@@ -37,7 +37,7 @@ my $TIMEOUT = 10;
 # Script can only grep for kernel addresses on the following architectures. If
 # your architecture is not listed here and has a grep'able kernel address 
please
 # consider submitting a patch.
-my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
+my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
 
 # Command line options.
 my $help = 0;
@@ -49,6 +49,12 @@ my $input_raw = "";  # Read raw results from file instead of 
scanning.
 my $suppress_dmesg = 0;# Don't show dmesg in output.
 my $squash_by_path = 0;# Summary report grouped by absolute 
path.
 my $squash_by_filename = 0;# Summary report grouped by filename.
+my $page_offset_param = 0;  # 32-bit: overrides value of PAGE_OFFSET_32BIT
+
+my $bit_size = 64;  # Check 64-bit kernel addresses by default
+my $kconfig_file = '/boot/config-'.`uname -r`;
+$kconfig_file =~ s/\R*//g;
+my $PAGE_OFFSET_32BIT = 0xc000;
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',
@@ -99,10 +105,11 @@ Options:
 
-o, --output-raw=  Save results for future processing.
-i, --input-raw=   Read results from file instead of scanning.
-   --rawShow 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.
+   --rawShow 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.
+   --page-offset=  PAGE_OFFSET value (for 32-bit kernels).
-d, --debug  Display debugging output.
-h, --help, --versionDisplay this help and exit.
 
@@ -117,7 +124,7 @@ Examples:
# View summary report.
$0 --input-raw scan.out --squash-by-filename
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+Scans the running (32 or 64 bit) kernel for potential leaking addresses.
 
 EOM
exit($exitcode);
@@ -133,10 +140,16 @@ GetOptions(
'squash-by-path'=> \$squash_by_path,
'squash-by-filename'=> \$squash_by_filename,
'raw'   => \$raw,
+   'page-offset=o' => \$page_offset_param,
 ) or help(1);
 
 help(0) if ($help);
 
+sub dprint
+{
+   printf(STDERR @_) if $debug;
+}
+
 if ($input_raw) {
format_output($input_raw);
exit(0);
@@ -162,6 +175,24 @@ if (!is_supported_architecture()) {
exit(129);
 }
 
+dprint "Detected arch : $bit_size bits\n";
+
+if ($bit_size == 32) {
+   # Parameter --page-offset passed? if Y, override with it
+   if ($page_offset_param != 0) {
+   $PAGE_OFFSET_32BIT = $page_offset_param;
+   } else {
+   $PAGE_OFFSET_32BIT = eval parse_kconfig($kconfig_file, 
"CONFIG_PAGE_OFFSET");
+   if ($PAGE_OFFSET_32BIT == 0) {
+   printf "$P: Fatal Error :: couldn't parse 
CONFIG_PAGE_OFFSET, aborting...\n";
+   printf " [Detail :: 

[PATCH 1/2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[kaiwan . billimoria]

The current leaking_addresses.pl script only supports showing "leaked"
64-bit kernel virtual addresses. This patch adds support for showing
"leaked" 32-bit kernel virtual addresses. It also takes into account Tobin's
feedback on the previous iteration. (Note: this patch is meant to apply on
the 'leaks' branch of Tobin's tree).

Briefly, the way it works- once it detects we're running on an i'x'86 platform,
(where x=3|4|5|6), it takes this arch into account for checking. The essential 
rationale:
 if virt-addr >= PAGE_OFFSET => it's a kernel virtual address.

This version programatically queries and sets PAGE_OFFSET based on the
/boot/config-$(uname -r) content. If, for any reason, this file cannot be
used, we fallback to requesting the user to pass PAGE_OFFSET as a parameter.

Pending/TODO:
- support for ARM-32

Feedback welcome..


Signed-off-by: Kaiwan N Billimoria 
---
diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 865c07649dff..0566f8055ec5 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -2,10 +2,10 @@
 #
 # (c) 2017 Tobin C. Harding 
 # (c) 2017 Kaiwan N Billimoria  (ix86 support)
- 
+
 # Licensed under the terms of the GNU GPL License version 2
 #
-# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
+# leaking_addresses.pl: Scan 32/64 bit kernel for potential leaking addresses.
 #  - Scans dmesg output.
 #  - Walks directory tree and parses each file (for each directory in @DIRS).
 #
@@ -14,7 +14,7 @@
 #
 # You may like to set kptr_restrict=2 before running script
 # (see Documentation/sysctl/kernel.txt).
-
+#
 use warnings;
 use strict;
 use POSIX;
@@ -37,7 +37,7 @@ my $TIMEOUT = 10;
 # Script can only grep for kernel addresses on the following architectures. If
 # your architecture is not listed here and has a grep'able kernel address 
please
 # consider submitting a patch.
-my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
+my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
 
 # Command line options.
 my $help = 0;
@@ -49,6 +49,12 @@ my $input_raw = "";  # Read raw results from file instead of 
scanning.
 my $suppress_dmesg = 0;# Don't show dmesg in output.
 my $squash_by_path = 0;# Summary report grouped by absolute 
path.
 my $squash_by_filename = 0;# Summary report grouped by filename.
+my $page_offset_param = 0;  # 32-bit: overrides value of PAGE_OFFSET_32BIT
+
+my $bit_size = 64;  # Check 64-bit kernel addresses by default
+my $kconfig_file = '/boot/config-'.`uname -r`;
+$kconfig_file =~ s/\R*//g;
+my $PAGE_OFFSET_32BIT = 0xc000;
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',
@@ -99,10 +105,11 @@ Options:
 
-o, --output-raw=  Save results for future processing.
-i, --input-raw=   Read results from file instead of scanning.
-   --rawShow 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.
+   --rawShow 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.
+   --page-offset=  PAGE_OFFSET value (for 32-bit kernels).
-d, --debug  Display debugging output.
-h, --help, --versionDisplay this help and exit.
 
@@ -117,7 +124,7 @@ Examples:
# View summary report.
$0 --input-raw scan.out --squash-by-filename
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+Scans the running (32 or 64 bit) kernel for potential leaking addresses.
 
 EOM
exit($exitcode);
@@ -133,10 +140,16 @@ GetOptions(
'squash-by-path'=> \$squash_by_path,
'squash-by-filename'=> \$squash_by_filename,
'raw'   => \$raw,
+   'page-offset=o' => \$page_offset_param,
 ) or help(1);
 
 help(0) if ($help);
 
+sub dprint
+{
+   printf(STDERR @_) if $debug;
+}
+
 if ($input_raw) {
format_output($input_raw);
exit(0);
@@ -162,6 +175,24 @@ if (!is_supported_architecture()) {
exit(129);
 }
 
+dprint "Detected arch : $bit_size bits\n";
+
+if ($bit_size == 32) {
+   # Parameter --page-offset passed? if Y, override with it
+   if ($page_offset_param != 0) {
+   $PAGE_OFFSET_32BIT = $page_offset_param;
+   } else {
+   $PAGE_OFFSET_32BIT = eval parse_kconfig($kconfig_file, 
"CONFIG_PAGE_OFFSET");
+   if ($PAGE_OFFSET_32BIT == 0) {
+   printf "$P: Fatal Error :: couldn't parse 
CONFIG_PAGE_OFFSET, aborting...\n";
+   printf " [Detail :: arch=32-bit, kconfig 
file=$kconfig_file]\n\n";
+   printf