linux-kernel - Fwd: [PATCH] x86/nmi/64: avoid passing user space rsp of pt

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

Message-ID: <CAK4g67Z6Mt4fDCF7h52BpaBGhokn-Y_MKtsihambRP86o5jBCA@mail.gmail.com>
Date:   Wed, 23 Aug 2017 01:39:47 +0800
From:   oliver yang <yangoliver@...il.com>
To:     linux-kernel@...r.kernel.org
Subject: Fwd: [PATCH] x86/nmi/64: avoid passing user space rsp of pt_regs to
 nmi handler

Copied linux-kernel, as my email got rejected by the alias.


---------- Forwarded message ----------
From: yang oliver <yang_oliver@...mail.com>
Date: 2017-08-23 1:09 GMT+08:00
Subject: [PATCH] x86/nmi/64: avoid passing user space rsp of pt_regs
to nmi handler
To: "tglx@...utronix.de" <tglx@...utronix.de>, "mingo@...hat.com"
<mingo@...hat.com>, "hpa@...or.com" <hpa@...or.com>, "luto@...nel.org"
<luto@...nel.org>, "x86@...nel.org" <x86@...nel.org>
抄送： "rostedt@...dmis.org" <rostedt@...dmis.org>, "jpoimboe@...hat.com"
<jpoimboe@...hat.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Yong Yang <yangoliver@...il.com>


From: Yong Yang <yangoliver@...il.com>

While NMI interrupts the very beginning of SYSCALL, the rsp could
be still user space address without loading kernel address. Then
the pt_regs constructed by the NMI entry point could have a user
space rsp. If a NMI handler tried to dump stack by using this rsp,
it can cause the kernel panic.

This patch tries to check the rsp saved in outermost frame, and
will do another copy for pt_regs. If the rsp of outermost frame
is from user space, then use the kernel address of the rsp as the
member of pt_regs.

Signed-off-by: Yong Yang <yangoliver@...il.com>
---
 arch/x86/entry/entry_64.S | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index d271fb7..dc85452 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1298,7 +1298,13 @@ ENTRY(nmi)
         * | outermost CS          } Copied to "iret" frame on each  |
         * | outermost RIP         } iteration.                      |
         * +---------------------------------------------------------+
-        * | pt_regs                                                 |
+        * | pt_regs SS          } initialized in first_nmi;         |
+        * | pt_regs Return RSP  } will not be changed before        |
+        * | pt_regs RFLAGS      } NMI processing is done.           |
+        * | pt_regs CS          } Passed to do_nmi for NMI handler. |
+        * | pt_regs RIP         } User space RSP could be replaced. |
+        * +---------------------------------------------------------+
+        * | pt_regs (remaining registers)                           |
         * +---------------------------------------------------------+
         *
         * The "original" frame is used by hardware.  Before re-enabling
@@ -1394,9 +1400,6 @@ nested_nmi_out:
        INTERRUPT_RETURN

 first_nmi:
-       /* Restore rdx. */
-       movq    (%rsp), %rdx
-
        /* Make room for "NMI executing". */
        pushq   $0

@@ -1408,6 +1411,24 @@ first_nmi:
        pushq   11*8(%rsp)
        .endr

+       /* Copy the "outermost" frame to pt_regs frame */
+       pushq   16*8(%rsp)
+       movq    $__PAGE_OFFSET, %rdx
+       cmpq    16*8(%rsp), %rdx        /* kernel rsp? */
+       jbe     2f
+       /* Save user RSP to avoid pt_regs->rsp got referenced by NMI handler */
+       pushq   %rsp
+       jmp     3f
+2:
+       pushq   16*8(%rsp)
+3:
+       .rept 3
+       pushq   16*8(%rsp)
+       .endr
+
+       /* Restore rdx. */
+       movq    16*8(%rsp), %rdx
+
        /* Everything up to here is safe from nested NMIs */

 #ifdef CONFIG_DEBUG_ENTRY
@@ -1441,18 +1462,18 @@ repeat_nmi:
         * gsbase if needed before we call do_nmi.  "NMI executing"
         * is zero.
         */
-       movq    $1, 10*8(%rsp)          /* Set "NMI executing". */
+       movq    $1, 15*8(%rsp)          /* Set "NMI executing". */

        /*
         * Copy the "outermost" frame to the "iret" frame.  NMIs that nest
         * here must not modify the "iret" frame while we're writing to
         * it or it will end up containing garbage.
         */
-       addq    $(10*8), %rsp
+       addq    $(15*8), %rsp
        .rept 5
        pushq   -6*8(%rsp)
        .endr
-       subq    $(5*8), %rsp
+       subq    $(10*8), %rsp
 end_repeat_nmi:

        /*
@@ -1486,7 +1507,7 @@ nmi_restore:
        RESTORE_C_REGS

        /* Point RSP at the "iret" frame. */
-       REMOVE_PT_GPREGS_FROM_STACK 6*8
+       REMOVE_PT_GPREGS_FROM_STACK 11*8

        /*
         * Clear "NMI executing".  Set DF first so that we can easily
--
1.8.3.1



-- 
------------------
Oliver Yang