Re: [RFC PATCH v2] x86/arch_prctl: Add ARCH_SET_XCR0 to set XCR0 per-thread

From: Dave Hansen <dave.hansen@intel.com>
To: Keno Fischer <keno@juliacomputing.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andi Kleen <andi@firstfloor.org>, Kyle Huey <khuey@kylehuey.com>,
	Robert O'Callahan <robert@ocallahan.org>
Subject: Re: [RFC PATCH v2] x86/arch_prctl: Add ARCH_SET_XCR0 to set XCR0 per-thread
Date: Tue, 7 Apr 2020 09:27:31 -0700	[thread overview]
Message-ID: <5208ad1e-cd9b-d57e-15b0-0ca935fccacd@intel.com> (raw)
In-Reply-To: <CABV8kRw1TQsqs+z43bSfZ5isctuFGMB4g_ztDYihiiXHcy4nVA@mail.gmail.com>

>> How does this work with things like xstateregs_[gs]et() where the format
>> of the kernel buffer and thus the kernel XCR0 is exposed as part of our
>> ABI?  With this patch, wouldn't a debugger app see a state buffer that
>> looks invalid?
> 
> Since those operate on the in-kernel buffer of these, which
> in this patch always uses the unmodified XCR0, ptracers
> should not observe a difference.

Those operate on *BOTH* kernel and userspace buffers.  They copy between
them.  That's kinda the point. :)

But I don't see any modifications to copy_xstate_to_user() or
user_regset_copyout() in your patch.

I suspect the patch thus far is only the tip of the iceberg.  I'd really
suggest doing some more thorough audits of all of the locations in the
kernel that touch the fpu buffer *or* that call XSAVE/XRSTOR.  I'm
pretty sure that audit hasn't been done or the ptrace example would have
been found already.

>> I'm also trying to think of what kinds of things CPU companies add to
>> their architectures that would break this stuff.  I can't recall ever
>> having a discussion with folks at Intel where we're designing a CPU
>> feature and we say, "Can't do that, it would break record/replay".
> 
> Heh, I'm having these discussions for you - ask me which Intel
> microarchitectures have interesting bugs here ;). The fact that rr works,
> is pretty much the only reason we buy Intel hardware these days, so
> there is at least a good reason for Intel folks to care. I think the evil
> plan is to make rr so good that everybody is using it, so you'll
> start having these conversations more :).

Having reverse execution is a laudable goal.  I've been using this:

	https://www.windriver.com/products/simics/

to do kernel (and occasional app) debugging the last few years, and its
reverse execution is invaluable for certain kinds of debugging.  But,
it's also not my daily go-to for debugging.

I'm just far from convinced that we your problem is worth solving,
especially in the place you're proposing to solve it.