linux-kernel - Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations

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

Message-ID: <CALCETrWJeNXiEPPVecc2OGRENj220zNeFCYgm+Q2P7ZiMvoVFg@mail.gmail.com>
Date:   Thu, 29 Jun 2017 11:50:18 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     X86 ML <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        live-patching@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>, Jiri Slaby <jslaby@...e.cz>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Mike Galbraith <efault@....de>
Subject: Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations

On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> There's a bug here that will need a small change to the entry code.
>
> Mike Galbraith reported:
>
>   WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb
>
> After some looking I found that it's caused by the following code
> snippet in the 'interrupt' macro in entry_64.S:
>
>         /*
>          * Save previous stack pointer, optionally switch to interrupt stack.
>          * irq_count is used to check if a CPU is already on an interrupt stack
>          * or not. While this is essentially redundant with preempt_count it is
>          * a little cheaper to use a separate counter in the PDA (short of
>          * moving irq_enter into assembly, which would be too much work)
>          */
>         movq    %rsp, %rdi
>         incl    PER_CPU_VAR(irq_count)
>         cmovzq  PER_CPU_VAR(irq_stack_ptr), %rsp
>         UNWIND_HINT_REGS base=rdi
>         pushq   %rdi
>         UNWIND_HINT_REGS indirect=1
>
> The problem is that it's changing the stack pointer *before* writing the
> previous stack pointer (push %rdi).  So when unwinding from an NMI which
> hit between the rsp write and the rdi push, the unwinder tries to access
> the regs on the previous stack (by reading rdi), but the previous stack
> pointer isn't there yet, so the access is considered out of bounds.

Ugh, that code.  Does this problem go away with this patch applied:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1

If so, want to update the patch for new kernels (shouldn't conflict
with anything except your unwind hints)?

--Andy