Re: [PATCH v2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Kaiwan N Billimoria]

On Tue, Nov 28, 2017 at 11:58 AM, Tobin C. Harding  wrote:
>
> At this stage I am unsure
> how best to convey my ideas back to you. It seems that adding 32 bit x86
> support is making a big enough change to the script that rather than you
> patching and me maintaining we could see it more as co-developing the
> patch.

That's just great! Thanks for the vote of confidence..

>I am in no way trying to take away from your changes, I'm happy
> for all work we end up with being applied with you as the author.
Of course not..

> If you are happy with this, I will email a patch to you (and CC
> kernel-hardening).
Yes..

>You could then look at it and see what things you
> like and what things you don't. Also I have not got access to a 32 bit
> x86 machine so it has not been tested. Once you are happy with it
> perhaps you could re-send as v3 and then I can apply it to the tree with
> you as the author.

Certainly, will do..

> The main aims of my changes to your patch are:
>
> 1. Keep inline with current script as much as possible.
> 2. Keep code as clean as possible (Perl can go to spaghetti really fast).
> 3. Try to keep the architecture stuff un-entangled, assuming more
>architecture specific code will be needed in the future.
>
> And now I'll add a few comments inline intended to add clarity to my
> patch when it comes.

With even a quick glance, I can see you've definitely improved readability and
such...

> Thanks Kaiwan. If any of my methods are unclear or you don't like them
> please do say. I'm hear to learn also, we are shooting for the best
> Kernel possible.
Absolutely! Thanks again..
...

> I played around with these two subs for a while. Let me know what you
> think.
>
> thanks,
> Tobin.

Will look into your patch in detail and revert..

thanks v much,
Kaiwan.

Re: [PATCH v2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Kaiwan N Billimoria]

On Tue, Nov 28, 2017 at 11:58 AM, Tobin C. Harding  wrote:
>
> At this stage I am unsure
> how best to convey my ideas back to you. It seems that adding 32 bit x86
> support is making a big enough change to the script that rather than you
> patching and me maintaining we could see it more as co-developing the
> patch.

That's just great! Thanks for the vote of confidence..

>I am in no way trying to take away from your changes, I'm happy
> for all work we end up with being applied with you as the author.
Of course not..

> If you are happy with this, I will email a patch to you (and CC
> kernel-hardening).
Yes..

>You could then look at it and see what things you
> like and what things you don't. Also I have not got access to a 32 bit
> x86 machine so it has not been tested. Once you are happy with it
> perhaps you could re-send as v3 and then I can apply it to the tree with
> you as the author.

Certainly, will do..

> The main aims of my changes to your patch are:
>
> 1. Keep inline with current script as much as possible.
> 2. Keep code as clean as possible (Perl can go to spaghetti really fast).
> 3. Try to keep the architecture stuff un-entangled, assuming more
>architecture specific code will be needed in the future.
>
> And now I'll add a few comments inline intended to add clarity to my
> patch when it comes.

With even a quick glance, I can see you've definitely improved readability and
such...

> Thanks Kaiwan. If any of my methods are unclear or you don't like them
> please do say. I'm hear to learn also, we are shooting for the best
> Kernel possible.
Absolutely! Thanks again..
...

> I played around with these two subs for a while. Let me know what you
> think.
>
> thanks,
> Tobin.

Will look into your patch in detail and revert..

thanks v much,
Kaiwan.

Re: [PATCH v2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Tobin C. Harding]

On Mon, Nov 27, 2017 at 08:42:16AM +0530, kaiwan.billimo...@gmail.com wrote:
> Currently, leaking_addresses.pl only supports scanning and displaying 'leaked'
> 64-bit kernel virtual addresses. We can scan for and display 'leaked' 32-bit
> kernel virtual addresses as well.

Hi Kaiwan,

This is starting to look good. I have a few suggestions and comments. I
applied your patch and played around with it. At this stage I am unsure
how best to convey my ideas back to you. It seems that adding 32 bit x86
support is making a big enough change to the script that rather than you
patching and me maintaining we could see it more as co-developing the
patch. I am in no way trying to take away from your changes, I'm happy
for all work we end up with being applied with you as the author.

The reasons for me doing this instead of just commenting inline is that

1. I'm not a pro Perl hacker, so commenting in line in English is prone
   to error.
2. I've only been a maintainer for a couple of weeks so I'm learning on
   the job.

If you are happy with this, I will email a patch to you (and CC
kernel-hardening). You could then look at it and see what things you
like and what things you don't. Also I have not got access to a 32 bit
x86 machine so it has not been tested. Once you are happy with it
perhaps you could re-send as v3 and then I can apply it to the tree with
you as the author.

We don't seem to be getting a lot of interest from the list but if any
other maintainers want to step in and school us, please do.

If any Perl mongers would like to correct us, also this would be
awesome.

The main aims of my changes to your patch are:

1. Keep inline with current script as much as possible.
2. Keep code as clean as possible (Perl can go to spaghetti really fast).
3. Try to keep the architecture stuff un-entangled, assuming more
   architecture specific code will be needed in the future.

And now I'll add a few comments inline intended to add clarity to my
patch when it comes.

Thanks Kaiwan. If any of my methods are unclear or you don't like them
please do say. I'm hear to learn also, we are shooting for the best
Kernel possible.

> 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 32-bit-virt-addr >= PAGE_OFFSET => it's a kernel virtual address.
> 
> This version programatically queries and sets PAGE_OFFSET based on it's value
> in one of these files: /boot/config, /boot/config-$(uname -r) and
> /proc/config.gz. If, for any reason, none of these files can be used, we
> fallback to requesting the user to pass PAGE_OFFSET as an option switch.

I re-wrote the commit log. Take it or leave it, as you please.

> Feedback welcome..
> 
> 
> Kaiwan N Billimoria (1):
>   scripts: leaking_addresses: add support for 32-bit kernel addresses
> 
>  scripts/leaking_addresses.pl | 150 
> +--
>  1 file changed, 132 insertions(+), 18 deletions(-)
> 
> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 2d5336b3e1ea..fccd0a5094f1 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -5,7 +5,7 @@
>  
>  # 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 the 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,9 @@ 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_32bit = 0;  # 32-bit: value of CONFIG_PAGE_OFFSET

We have to be super careful with spaces and tabs. When things like this
appear I like to make whitespace visible in the editor so we can see
what is going on.

> +my @kernel_config_files = ('/boot/config', '/boot/config-'.`uname -r`, 
> '/proc/config.gz');

I moved this into get_page_offset. Also I added a command line option 

Re: [PATCH v2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Tobin C. Harding]

On Mon, Nov 27, 2017 at 08:42:16AM +0530, kaiwan.billimo...@gmail.com wrote:
> Currently, leaking_addresses.pl only supports scanning and displaying 'leaked'
> 64-bit kernel virtual addresses. We can scan for and display 'leaked' 32-bit
> kernel virtual addresses as well.

Hi Kaiwan,

This is starting to look good. I have a few suggestions and comments. I
applied your patch and played around with it. At this stage I am unsure
how best to convey my ideas back to you. It seems that adding 32 bit x86
support is making a big enough change to the script that rather than you
patching and me maintaining we could see it more as co-developing the
patch. I am in no way trying to take away from your changes, I'm happy
for all work we end up with being applied with you as the author.

The reasons for me doing this instead of just commenting inline is that

1. I'm not a pro Perl hacker, so commenting in line in English is prone
   to error.
2. I've only been a maintainer for a couple of weeks so I'm learning on
   the job.

If you are happy with this, I will email a patch to you (and CC
kernel-hardening). You could then look at it and see what things you
like and what things you don't. Also I have not got access to a 32 bit
x86 machine so it has not been tested. Once you are happy with it
perhaps you could re-send as v3 and then I can apply it to the tree with
you as the author.

We don't seem to be getting a lot of interest from the list but if any
other maintainers want to step in and school us, please do.

If any Perl mongers would like to correct us, also this would be
awesome.

The main aims of my changes to your patch are:

1. Keep inline with current script as much as possible.
2. Keep code as clean as possible (Perl can go to spaghetti really fast).
3. Try to keep the architecture stuff un-entangled, assuming more
   architecture specific code will be needed in the future.

And now I'll add a few comments inline intended to add clarity to my
patch when it comes.

Thanks Kaiwan. If any of my methods are unclear or you don't like them
please do say. I'm hear to learn also, we are shooting for the best
Kernel possible.

> 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 32-bit-virt-addr >= PAGE_OFFSET => it's a kernel virtual address.
> 
> This version programatically queries and sets PAGE_OFFSET based on it's value
> in one of these files: /boot/config, /boot/config-$(uname -r) and
> /proc/config.gz. If, for any reason, none of these files can be used, we
> fallback to requesting the user to pass PAGE_OFFSET as an option switch.

I re-wrote the commit log. Take it or leave it, as you please.

> Feedback welcome..
> 
> 
> Kaiwan N Billimoria (1):
>   scripts: leaking_addresses: add support for 32-bit kernel addresses
> 
>  scripts/leaking_addresses.pl | 150 
> +--
>  1 file changed, 132 insertions(+), 18 deletions(-)
> 
> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 2d5336b3e1ea..fccd0a5094f1 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -5,7 +5,7 @@
>  
>  # 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 the 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,9 @@ 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_32bit = 0;  # 32-bit: value of CONFIG_PAGE_OFFSET

We have to be super careful with spaces and tabs. When things like this
appear I like to make whitespace visible in the editor so we can see
what is going on.

> +my @kernel_config_files = ('/boot/config', '/boot/config-'.`uname -r`, 
> '/proc/config.gz');

I moved this into get_page_offset. Also I added a command line option 
--kernel-cofig-file.

>  # Do not parse 

[PATCH v2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[kaiwan . billimoria]

Currently, leaking_addresses.pl only supports scanning and displaying 'leaked'
64-bit kernel virtual addresses. We can scan for and display 'leaked' 32-bit
kernel virtual addresses as well.

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 32-bit-virt-addr >= PAGE_OFFSET => it's a kernel virtual address.

This version programatically queries and sets PAGE_OFFSET based on it's value
in one of these files: /boot/config, /boot/config-$(uname -r) and
/proc/config.gz. If, for any reason, none of these files can be used, we
fallback to requesting the user to pass PAGE_OFFSET as an option switch.

Feedback welcome..


Kaiwan N Billimoria (1):
  scripts: leaking_addresses: add support for 32-bit kernel addresses

 scripts/leaking_addresses.pl | 150 +--
 1 file changed, 132 insertions(+), 18 deletions(-)

Signed-off-by: Kaiwan N Billimoria 
---
diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 2d5336b3e1ea..fccd0a5094f1 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -5,7 +5,7 @@
 
 # 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 the 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,9 @@ 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_32bit = 0;  # 32-bit: value of CONFIG_PAGE_OFFSET
+
+my @kernel_config_files = ('/boot/config', '/boot/config-'.`uname -r`, 
'/proc/config.gz');
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',
@@ -97,14 +100,15 @@ Version: $V
 
 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.
-   -d, --debug  Display debugging output.
-   -h, --help, --versionDisplay this help and exit.
+   -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.
+   --page-offset-32bit=  PAGE_OFFSET value (for 32-bit kernels).
+   -d, --debugDisplay debugging output.
+   -h, --help, --version  Display this help and exit.
 
 Examples:
 
@@ -117,7 +121,11 @@ Examples:
# View summary report.
$0 --input-raw scan.out --squash-by-filename
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+   # (On a 32-bit system with a 2GB:2GB VMSPLIT), pass PAGE_OFFSET value
+   # as an option switch.
+   $0 --page-offset-32bit=0x8000
+
+Scans the running kernel for potential leaking addresses.
 
 EOM
exit($exitcode);
@@ -133,10 +141,16 @@ GetOptions(
'squash-by-path'=> \$squash_by_path,
'squash-by-filename'=> \$squash_by_filename,
'raw'   => \$raw,
+   'page-offset-32bit=o'   => \$page_offset_32bit,
 ) or help(1);
 
 help(0) if ($help);
 
+sub dprint
+{
+   printf(STDERR @_) if $debug;
+}
+
 if ($input_raw) {
format_output($input_raw);
exit(0);
@@ -162,6 +176,20 @@ if (!is_supported_architecture()) {
exit(129);
 }
 
+if ($debug) {
+   printf "Detected arch : ";
+   if (is_ix86_32()) {
+   printf "32 bit x86\n";
+   } else {
+   printf "64 bit\n";
+   }
+}
+
+if 

[PATCH v2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[kaiwan . billimoria]

Currently, leaking_addresses.pl only supports scanning and displaying 'leaked'
64-bit kernel virtual addresses. We can scan for and display 'leaked' 32-bit
kernel virtual addresses as well.

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 32-bit-virt-addr >= PAGE_OFFSET => it's a kernel virtual address.

This version programatically queries and sets PAGE_OFFSET based on it's value
in one of these files: /boot/config, /boot/config-$(uname -r) and
/proc/config.gz. If, for any reason, none of these files can be used, we
fallback to requesting the user to pass PAGE_OFFSET as an option switch.

Feedback welcome..


Kaiwan N Billimoria (1):
  scripts: leaking_addresses: add support for 32-bit kernel addresses

 scripts/leaking_addresses.pl | 150 +--
 1 file changed, 132 insertions(+), 18 deletions(-)

Signed-off-by: Kaiwan N Billimoria 
---
diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
index 2d5336b3e1ea..fccd0a5094f1 100755
--- a/scripts/leaking_addresses.pl
+++ b/scripts/leaking_addresses.pl
@@ -5,7 +5,7 @@
 
 # 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 the 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,9 @@ 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_32bit = 0;  # 32-bit: value of CONFIG_PAGE_OFFSET
+
+my @kernel_config_files = ('/boot/config', '/boot/config-'.`uname -r`, 
'/proc/config.gz');
 
 # Do not parse these files (absolute path).
 my @skip_parse_files_abs = ('/proc/kmsg',
@@ -97,14 +100,15 @@ Version: $V
 
 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.
-   -d, --debug  Display debugging output.
-   -h, --help, --versionDisplay this help and exit.
+   -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.
+   --page-offset-32bit=  PAGE_OFFSET value (for 32-bit kernels).
+   -d, --debugDisplay debugging output.
+   -h, --help, --version  Display this help and exit.
 
 Examples:
 
@@ -117,7 +121,11 @@ Examples:
# View summary report.
$0 --input-raw scan.out --squash-by-filename
 
-Scans the running (64 bit) kernel for potential leaking addresses.
+   # (On a 32-bit system with a 2GB:2GB VMSPLIT), pass PAGE_OFFSET value
+   # as an option switch.
+   $0 --page-offset-32bit=0x8000
+
+Scans the running kernel for potential leaking addresses.
 
 EOM
exit($exitcode);
@@ -133,10 +141,16 @@ GetOptions(
'squash-by-path'=> \$squash_by_path,
'squash-by-filename'=> \$squash_by_filename,
'raw'   => \$raw,
+   'page-offset-32bit=o'   => \$page_offset_32bit,
 ) or help(1);
 
 help(0) if ($help);
 
+sub dprint
+{
+   printf(STDERR @_) if $debug;
+}
+
 if ($input_raw) {
format_output($input_raw);
exit(0);
@@ -162,6 +176,20 @@ if (!is_supported_architecture()) {
exit(129);
 }
 
+if ($debug) {
+   printf "Detected arch : ";
+   if (is_ix86_32()) {
+   printf "32 bit x86\n";
+   } else {
+   printf "64 bit\n";
+   }
+}
+
+if (is_ix86_32()) {
+