Re: CVE-2015-5157 IRET faults during NMIs processing vs 3.10

Re: CVE-2015-5157 IRET faults during NMIs processing vs 3.10

[Andy Lutomirski]

Hi Greg-

Jiri has had these patches applied to 3.12 for a while.  Can you
backport to 3.10?  See below.

Thanks,
Andy

On Thu, Oct 29, 2015 at 12:05 PM, Mateusz Guzik <mguzik@redhat.com> wrote:
> On Wed, Oct 14, 2015 at 08:29:21AM -0700, Andy Lutomirski wrote:
>> On Wed, Oct 14, 2015 at 5:19 AM, Mateusz Guzik <mguzik@redhat.com> wrote:
>> > Hello,
>> >
>> > the bug in question is present in 3.10 kernels, which happen to be used
>> > by rhel 7 and I was told to patch this up.
>> >
>> > If I read the code right patchset you applied to 3.12 can be taken
>> > as-is, and from my testing it indeed works fine.
>> >
>> > Since the company likes to have stuff fixed in upstream, would you be
>> > willing to backport said patches to 3.10 as well? As noted earlier, they
>> > just apply so that should be easy.
>> >
>> > There is also a nit with missing 83c133cf11fb0e68a51 SWAPGS ->
>> > SWAPGS_UNSAFE_STACK in 3.12 and possibly other branches.
>> >
>> > That said, I can post the patchset myself, but all patches are authored
>> > by you and I presume would have to wait for your review anyway so that
>> > does not seem to save you any time if you would be willing to have a
>> > look.
>> >
>>
>> I'm pretty swamped right now, but I could take a look at proposed
>> patches.  I can also send you some tests to run.
>>
>> The sigreturn_32 test from a modern upstream kernel tree is a decent
>> test for some of this stuff, and the exploits I sent out are pretty
>> good, too.
>>
>
> Sorry for late reply.
>
> Maybe I was not precise enough, but I have run the exploit provided in
> http://www.openwall.com/lists/oss-security/2015/07/22/7 and it stopped
> panicking the kernel on a stock v3.10 patched with stuff below.
>
> I was not aware of sigreturn_32, it passed just fine.
>
> I'm happy to run whatever tests you can provide.
>
> Given that there was no need to modify patches (apart from entry_64.S
> location) and that I don't really know the stable submission process,
> let me just point them here (which likely i should have done in the initial
> mail, sorry):

You're missing:

commit fc57a7c68020dcf954428869eafd934c0ab1536f
Author: Andy Lutomirski <luto@kernel.org>
Date:   Sun Sep 20 16:32:04 2015 -0700

    x86/paravirt: Replace the paravirt nop with a bona fide empty function

Otherwise this sounds good.

>
> commit 83c133cf11fb0e68a51681447e372489f052d40e
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Sun Sep 20 16:32:05 2015 -0700
>
>     x86/nmi/64: Fix a paravirt stack-clobbering bug in the NMI code
>
>     The NMI entry code that switches to the normal kernel stack needs to
>     be very careful not to clobber any extra stack slots on the NMI
>     stack.  The code is fine under the assumption that SWAPGS is just a
>     normal instruction, but that assumption isn't really true.  Use
>     SWAPGS_UNSAFE_STACK instead.
>
>     This is part of a fix for some random crashes that Sasha saw.
>
> ==================================================
> Applied on top of the following 3.12 patches taken verbatim:
>
> commit 864c198bbd4e091577602ad42016ccc835af3b93
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Wed Jul 15 10:29:38 2015 -0700
>
>     x86/nmi/64: Use DF to avoid userspace RSP confusing nested NMI detection
>
>     commit 810bc075f78ff2c221536eb3008eac6a492dba2d upstream.
>
>     We have a tricky bug in the nested NMI code: if we see RSP
>     pointing to the NMI stack on NMI entry from kernel mode, we
>     assume that we are executing a nested NMI.
>
>     This isn't quite true.  A malicious userspace program can point
>     RSP at the NMI stack, issue SYSCALL, and arrange for an NMI to
>     happen while RSP is still pointing at the NMI stack.
>
>     Fix it with a sneaky trick.  Set DF in the region of code that
>     the RSP check is intended to detect.  IRET will clear DF
>     atomically.
>
>     ( Note: other than paravirt, there's little need for all this
>       complexity. We could check RIP instead of RSP. )
>
> commit 10c99c9766cda45d7ff6cfba80d459f054308816
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Wed Jul 15 10:29:37 2015 -0700
>
>     x86/nmi/64: Reorder nested NMI checks
>
>     commit a27507ca2d796cfa8d907de31ad730359c8a6d06 upstream.
>
>     Check the repeat_nmi .. end_repeat_nmi special case first.  The
>     next patch will rework the RSP check and, as a side effect, the
>     RSP check will no longer detect repeat_nmi .. end_repeat_nmi, so
>     we'll need this ordering of the checks.
>
>     Note: this is more subtle than it appears.  The check for
>     repeat_nmi .. end_repeat_nmi jumps straight out of the NMI code
>     instead of adjusting the "iret" frame to force a repeat.  This
>     is necessary, because the code between repeat_nmi and
>     end_repeat_nmi sets "NMI executing" and then writes to the
>     "iret" frame itself.  If a nested NMI comes in and modifies the
>     "iret" frame while repeat_nmi is also modifying it, we'll end up
>     with garbage.  The old code got this right, as does the new
>     code, but the new code is a bit more explicit.
>
>     If we were to move the check right after the "NMI executing"
>     check, then we'd get it wrong and have random crashes.
>
>     ( Because the "NMI executing" check would jump to the code that would
>       modify the "iret" frame without checking if the interrupted NMI was
>       currently modifying it. )
>
> commit d2ad20321da32a57006e1ffca020e6e84b2b0e66
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Wed Jul 15 10:29:36 2015 -0700
>
>     x86/nmi/64: Improve nested NMI comments
>
>     commit 0b22930ebad563ae97ff3f8d7b9f12060b4c6e6b upstream.
>
>     I found the nested NMI documentation to be difficult to follow.
>     Improve the comments.
>
> commit e0de15fc45a83f94d1ef578f54b427b86a33ab21
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Wed Jul 15 10:29:35 2015 -0700
>
>     x86/nmi/64: Switch stacks on userspace NMI entry
>
>     commit 9b6e6a8334d56354853f9c255d1395c2ba570e0a upstream.
>
>     Returning to userspace is tricky: IRET can fail, and ESPFIX can
>     rearrange the stack prior to IRET.
>
>     The NMI nesting fixup relies on a precise stack layout and
>     atomic IRET.  Rather than trying to teach the NMI nesting fixup
>     to handle ESPFIX and failed IRET, punt: run NMIs that came from
>     user mode on the normal kernel stack.
>
>     This will make some nested NMIs visible to C code, but the C
>     code is okay with that.
>
>     As a side effect, this should speed up perf: it eliminates an
>     RDMSR when NMIs come from user mode.
>
> Thanks,
> --
> Mateusz Guzik



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: CVE-2015-5157 IRET faults during NMIs processing vs 3.10

[Greg KH]

On Thu, Oct 29, 2015 at 01:21:58PM -0700, Andy Lutomirski wrote:
> Hi Greg-
> 
> Jiri has had these patches applied to 3.12 for a while.  Can you
> backport to 3.10?  See below.

Ok, will do, I'll take some time next week to do this.

thanks,

greg k-h

Re: CVE-2015-5157 IRET faults during NMIs processing vs 3.10

[Mateusz Guzik]

On Thu, Oct 29, 2015 at 02:46:30PM -0700, Greg KH wrote:
> On Thu, Oct 29, 2015 at 01:21:58PM -0700, Andy Lutomirski wrote:
> > Hi Greg-
> > 
> > Jiri has had these patches applied to 3.12 for a while.  Can you
> > backport to 3.10?  See below.
> 
> Ok, will do, I'll take some time next week to do this.
> 

Did you get the chance to work on this? Sorry if I somehow missed the
patchset.

Thanks,
-- 
Mateusz Guzik

Re: CVE-2015-5157 IRET faults during NMIs processing vs 3.10

[Greg KH]

On Mon, Nov 09, 2015 at 09:31:26PM +0100, Mateusz Guzik wrote:
> On Thu, Oct 29, 2015 at 02:46:30PM -0700, Greg KH wrote:
> > On Thu, Oct 29, 2015 at 01:21:58PM -0700, Andy Lutomirski wrote:
> > > Hi Greg-
> > > 
> > > Jiri has had these patches applied to 3.12 for a while.  Can you
> > > backport to 3.10?  See below.
> > 
> > Ok, will do, I'll take some time next week to do this.
> > 
> 
> Did you get the chance to work on this? Sorry if I somehow missed the
> patchset.

Nope, not yet, if anyone wants to give me the patches in a nice tested
series, I would love it :)

thanks,

greg k-h