linux-kernel - Re: Investigating a stack state mismatch in Linux kernel

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-next>] [day] [month] [year] [list]

Message-ID: <2a2b1181-2175-7e4e-189c-12630f43d10d@linux.com>
Date:   Thu, 15 Nov 2018 23:51:13 +0300
From:   Alexander Popov <alex.popov@...ux.com>
To:     Florian Weimer <fweimer@...hat.com>,
        Richard Sandiford <richard.sandiford@....com>
Cc:     Kees Cook <keescook@...omium.org>, Ingo Molnar <mingo@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Tycho Andersen <tycho@...ho.ws>,
        Laura Abbott <labbott@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Emese Revfy <re.emese@...il.com>,
        Thomas Garnier <thgarnie@...gle.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Steven Rostedt <rostedt@...dmis.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Will Deacon <will.deacon@....com>,
        Jann Horn <jannh@...gle.com>,
        linux-arm-kernel@...ts.infradead.org, gcc-bugs@....gnu.org,
        gcc-help@....gnu.org, LKML <linux-kernel@...r.kernel.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>
Subject: Re: Investigating a stack state mismatch in Linux kernel

On 15.11.2018 13:24, Florian Weimer wrote:
> * Alexander Popov:
> 
>> Of course, there is a naive solution for this issue -- just skip stackleak
>> instrumentation for acpi_duplicate_processor_id(). But it would be great to find
>> out the reasons behind this compiler behavior. It might help to create a better
>> solution.
> 
> Please show us the RTL dumps with both compilers, both before and after
> the plugin pass.

Thanks a lot for your reply, Florian!

I have more information to share.

I attach the list of gcc passes. The 'rtl-stackleak_cleanup' runs after the
'rtl-reload' pass.

I attach RTL dumps both for gcc-5 and gcc-7 for:
  - 'rtl-reload' pass,
  - 'rtl-stackleak_cleanup' pass,
  - 'rtl-pro_and_epilogue' pass.

I've found out that for gcc-5:
  - if I put the 'rtl-stackleak_cleanup' pass (deleting CALL insn) *before* the
'rtl-pro_and_epilogue' pass, objtool reports about stack state mismatch in
acpi_duplicate_processor_id();
  - if I put the 'rtl-stackleak_cleanup' pass *after* the 'rtl-pro_and_epilogue'
pass, objtool *doesn't* report about stack state mismatch.

So gcc-5 does some mistake during the 'rtl-pro_and_epilogue' pass. And gcc-7
doesn't have this issue.

In the original grsecurity code the stackleak RTL pass was registered just
before the 'rtl-final' pass. Some time ago Richard Sandiford noted that:

>>> This might be too late, since it happens e.g. after addresses have
>>> been calculated for branch ranges, and after machine-specific passes
>>> (e.g. bundling on ia64).
>>>
>>> The stack size is final after reload, so inserting the pass after that
>>> might be better.

https://lore.kernel.org/patchwork/patch/879912/


So what is the best moment when we know the stack frame size and can safely
delete the CALL insn using delete_insn_and_edges()?

Thanks!

Best regards,
Alexander


View attachment "gcc5-after-stackleak_cleanup.txt" of type "text/plain" (8078 bytes)

View attachment "gcc5-after-reload.txt" of type "text/plain" (16996 bytes)

View attachment "gcc5-after-pro_and_epilogue.txt" of type "text/plain" (10318 bytes)

View attachment "gcc7-after-stackleak_cleanup.txt" of type "text/plain" (8192 bytes)

View attachment "gcc7-after-reload.txt" of type "text/plain" (19210 bytes)

View attachment "gcc7-after-pro_and_epilogue.txt" of type "text/plain" (11195 bytes)

View attachment "gcc-passes.txt" of type "text/plain" (20714 bytes)