* Re: CVE-2015-5157 IRET faults during NMIs processing vs 3.10
[not found] ` <20151029190550.GA12947@mguzik>
@ 2015-10-29 20:21 ` Andy Lutomirski
2015-10-29 21:46 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Andy Lutomirski @ 2015-10-29 20:21 UTC (permalink / raw)
To: Mateusz Guzik; +Cc: Greg KH, stable, Jiri Slaby
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
^ permalink raw reply [flat|nested] 4+ messages in thread