Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH v2 5/7] x86/mm, tracing: Fix CR2 corruption
From:   Andy Lutomirski <luto@amacapital.net>
In-Reply-To: <CAHk-=wjOFXr83=A03ORuT5qq5yiQPwN9vMOrYvTMcqdC-wNn8w@mail.gmail.com>
Date:   Sat, 6 Jul 2019 18:36:59 -0600
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@kernel.org>,
        Andrew Lutomirski <luto@kernel.org>,
        Peter Anvin <hpa@zytor.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Juergen Gross <jgross@suse.com>,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
        He Zhe <zhe.he@windriver.com>,
        Joel Fernandes <joel@joelfernandes.org>, devel@etsukata.com
Content-Transfer-Encoding: quoted-printable
Message-Id: <EBA737F7-43F3-448B-B251-EF6F0060DD12@amacapital.net>
References: <20190704195555.580363209@infradead.org> <20190704200050.534802824@infradead.org> <CAHk-=wiJ4no+TW-8KTfpO-Q5+aaTGVoBJzrnFTvj_zGpVbrGfA@mail.gmail.com> <20190705134916.GU3402@hirez.programming.kicks-ass.net> <CAHk-=whsgA+8XtqJY91gqHhh9xLYQLM3kLLFTby=uf2eoZyK7Q@mail.gmail.com> <20190706182728.435a89ed@gandalf.local.home> <CAHk-=wj=vCn1c7O4rpjwnS1fZbEppkeUhAq=ob3+wox0FKNZwQ@mail.gmail.com> <CAHk-=wjOFXr83=A03ORuT5qq5yiQPwN9vMOrYvTMcqdC-wNn8w@mail.gmail.com>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Jul 6, 2019, at 6:08 PM, Linus Torvalds <torvalds@linux-foundation.org>=
 wrote:
>=20
> On Sat, Jul 6, 2019 at 3:41 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>=20
>>> On Sat, Jul 6, 2019 at 3:27 PM Steven Rostedt <rostedt@goodmis.org> wrot=
e:
>>>=20
>>> We also have to deal with reading vmalloc'd data as that can fault too.
>>=20
>> Ahh, that may be a better reason for PeterZ's patches and reading cr2
>> very early from asm code than the stack trace case.
>=20
> Hmm. Another alternative might be to simply just make our vmalloc page
> fault handling more robust.
>=20
> 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.
>=20
> 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.
>=20
> The only reason it's fatal right now is that we care much too deeply about=

>=20
> (a) the error code
> (b) the pt_regs state
>=20
> when we handle vmalloc faults.
>=20
> 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.
>=20
> 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.
>=20
> 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, you=
r patch won=E2=80=99t do so well if the fault is from user mode, I think.


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