linux-kernel - RE:(2) [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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 07 May 2020 19:02:20 +0530
From:   Vaneet Narang <v.narang@...sung.com>
To:     Masahiro Yamada <masahiroy@...nel.org>,
        Maninder Singh <maninder1.s@...sung.com>
CC:     "George G. Davis" <george_davis@...tor.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        AMIT SAHRAWAT <a.sahrawat@...sung.com>,
        Arnd Bergmann <arnd@...db.de>, Andi Kleen <ak@...ux.intel.com>
Subject: RE:(2) [PATCH 3/4] scripts/checkstack.pl: add arm push handling for
 stack usage

Hi Masahiro, 

>> 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
  
 
> Do you know CONFIG_FRAME_WARN?
 Yes we know this and we use it to get compilation error if some function is using more stack.
This config will report issue at compilation.
 
>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.
 
We feel purpose of this patch is different from CONFIG_FRAME_WARN.
This patch is specific to ARM and fixes bug in stack usage calculation.

We were comparing stack usage of ARM with ARM64 and found big gap.
We realised ARM is not calculating stack usage properly.
It only considers stack used by local variables but it doesn't consider 
stack used to store register context at the start of functions. 
This is not the case with ARM64. It seems ARM64 considers both.

We found even stack variables are of same size on both target but 
arm64 stack usage is high.

Considering below assembly, Actual stack usage is 492 but current script reports 448.
push instruction uses 44 bytes of stack to take backup of registers as per ARM calling
convention.

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

Thanks & Regards,
Vaneet Narang