linux-kernel - Re: [PATCH v2 5/7] x86/mm, tracing: Fix CR2 corruption

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 6 Jul 2019 18:36:59 -0600
From:   Andy Lutomirski <luto@...capital.net>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...nel.org>,
        Andrew Lutomirski <luto@...nel.org>,
        Peter Anvin <hpa@...or.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Juergen Gross <jgross@...e.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        He Zhe <zhe.he@...driver.com>,
        Joel Fernandes <joel@...lfernandes.org>, devel@...ukata.com
Subject: Re: [PATCH v2 5/7] x86/mm, tracing: Fix CR2 corruption



> On Jul 6, 2019, at 6:08 PM, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> On Sat, Jul 6, 2019 at 3:41 PM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>> 
>>> On Sat, Jul 6, 2019 at 3:27 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>>> 
>>> We also have to deal with reading vmalloc'd data as that can fault too.
>> 
>> Ahh, that may be a better reason for PeterZ's patches and reading cr2
>> very early from asm code than the stack trace case.
> 
> Hmm. Another alternative might be to simply just make our vmalloc page
> fault handling more robust.
> 
> Right now, if we take a vmalloc page fault in an inconvenient spot, it
> is fatal because it corrupts the cr2 in the outer context.
> 
> However, it doesn't *need* to be fatal. Who cares if the outer context
> cr2 gets corrupted? We probably *shouldn't* care - it's an odd and
> unusual case, and the outer context could just handle the wrong
> vmalloc-address cr2 fine (it's going to be a no-op, since the inner
> page fault will have handled it already), return, and then re-fault.
> 
> The only reason it's fatal right now is that we care much too deeply about
> 
> (a) the error code
> (b) the pt_regs state
> 
> when we handle vmalloc faults.
> 
> So one option is that we simply handle the vmalloc faults _without_
> caring about the error code and the pt_regs state, because even if the
> error code or the pt_regs implies that the fault comes from user
> space, the cr2 value might be due to a vmalloc fault from the inner
> kernel page fault that corrupted cr2.
> 
> Right now vmalloc faults are already special and need to be handled
> without holding any locks etc. We'd just make them even more special,
> and say that we might have a vmalloc area fault from any context.
> 
> IOW, somethinig like the attached patch would make nesting vmalloc
> faults harmless. Sure, we'll do the "vmalloc fault" twice, and return
> and re-do the original page fault, but this is a very unusual case, so
> from a performance angle we don't really care.

Eww. I would like to be able to rely on fault into being correct.  Also, your patch won’t do so well if the fault is from user mode, I think.


> 
> But I guess the "read cr2 early" is fine too..
> 
>               Linus
> <patch.diff>