linux-kernel - Re: [PATCH 3/4] scripts/checkstack.pl: add arm push handling for stack usage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNAQxOtjsrS6b8pF93=0yqnTJT6b7V2hDOWsYGH-njNcZdw@mail.gmail.com>
Date:   Thu, 7 May 2020 19:35:29 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Maninder Singh <maninder1.s@...sung.com>
Cc:     "George G. Davis" <george_davis@...tor.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        a.sahrawat@...sung.com, Vaneet Narang <v.narang@...sung.com>,
        Arnd Bergmann <arnd@...db.de>, Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 3/4] scripts/checkstack.pl: add arm push handling for
 stack usage

(+CC Andi Kleen, and more mentor)


On Thu, Apr 30, 2020 at 9:50 PM Maninder Singh <maninder1.s@...sung.com> wrote:
>
> To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
> if FRAME POINTER is enabled.
> e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
>
> c01f0d50 <Y>:
> c01f0d44:       e1a0c00d        mov     ip, sp
> c01f0d48:       e92ddff0        push    {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
> c01f0d4c:       e24cb004        sub     fp, ip, #4
> c01f0d50:       e24dd094        sub     sp, sp, #448    ; 0x1C0
>
> $ cat dump | scripts/checkstack.pl arm
> 0xc01f0d50 Y []:                                        448
>
> added subroutine frame work for this.
> After change:
> 0xc01f0d500 Y []:                                       492
>
> Signed-off-by: Vaneet Narang <v.narang@...sung.com>
> Signed-off-by: Maninder Singh <maninder1.s@...sung.com>



I do not mean to block this patch, but
some people still invest efforts to improve this script.
So, please let me ask this question.

Do you know CONFIG_FRAME_WARN?

Commit 35bb5b1e0e84cfa1a8906f7e6a77f391ff315791
mentioned -Wframe-larger-than should obsolete
make checkstack.

Now, -Wframe-larger-than is supported by
both minimal version of GCC and Clang.

I know checkstack.pl dumps the stack size
of functions, which is different from what
-Wframe-larger-than does, but the goal is
quite similar, I think.

I just wondered if we need both.




> ---
>  scripts/checkstack.pl | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
> index 8e5ef98..b292ef4 100755
> --- a/scripts/checkstack.pl
> +++ b/scripts/checkstack.pl
> @@ -34,8 +34,10 @@ use strict;
>  # $& (whole re) matches the complete objdump line with the stack growth
>  # $1 (first bracket) matches the dynamic amount of the stack growth
>  #
> +# $sub: subroutine for special handling to check stack usage.
> +#
>  # use anything else and feel the pain ;)
> -my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
> +my (@stack, $re, $dre, $sub, $x, $xs, $funcre, $min_stack);
>  {
>         my $arch = shift;
>         if ($arch eq "") {
> @@ -59,6 +61,7 @@ my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
>         } elsif ($arch eq 'arm') {
>                 #c0008ffc:      e24dd064        sub     sp, sp, #100    ; 0x64
>                 $re = qr/.*sub.*sp, sp, #(([0-9]{2}|[3-9])[0-9]{2})/o;
> +               $sub = \&arm_push_handling;
>         } elsif ($arch =~ /^x86(_64)?$/ || $arch =~ /^i[3456]86$/) {
>                 #c0105234:       81 ec ac 05 00 00       sub    $0x5ac,%esp
>                 # or
> @@ -112,6 +115,24 @@ my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
>  }
>
>  #
> +# To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
> +# if FRAME POINTER is enabled.
> +# e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
> +#
> +sub arm_push_handling {
> +       my $regex = qr/.*push.*fp, ip, lr, pc}/o;
> +       my $size = 0;
> +       my $line_arg = shift;
> +
> +       if ($line_arg =~ m/$regex/) {
> +               $size = $line_arg =~ tr/,//;
> +               $size = ($size + 1) * 4;
> +       }
> +
> +       return $size;
> +}
> +
> +#
>  # main()
>  #
>  my ($func, $file, $lastslash, $total_size, $addr, $intro);
> @@ -163,6 +184,10 @@ while (my $line = <STDIN>) {
>                 $size = hex($size) if ($size =~ /^0x/);
>                 $total_size = $total_size + $size
>         }
> +       elsif (defined $sub) {
> +               my $size = &$sub($line);
> +               $total_size = $total_size + $size;


                  $total_size += $size;

           Or

                  $total_size += &$sub($line);

                   then, delete $size.



> +       }
>  }
>  if ($total_size > $min_stack) {
>         push @stack, "$intro$total_size\n";
> --
> 1.9.1
>


-- 
Best Regards
Masahiro Yamada