Re: [PATCH v9 00/17] Enable FSGSBASE instructions

From: Sasha Levin <sashal@kernel.org>
To: "Bae, Chang Seok" <chang.seok.bae@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	"Metzger, Markus T" <markus.t.metzger@intel.com>,
	Andi Kleen <ak@linux.intel.com>, "hpa@zytor.com" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"bp@alien8.de" <bp@alien8.de>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"Luck, Tony" <tony.luck@intel.com>,
	Pedro Alves <palves@redhat.com>, Simon Marchi <simark@simark.ca>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v9 00/17] Enable FSGSBASE instructions
Date: Mon, 13 Apr 2020 16:03:36 -0400	[thread overview]
Message-ID: <20200413200336.GA11216@sasha-vm> (raw)
In-Reply-To: <29FD6626-4347-4B79-A027-52E44C7FDE55@intel.com>

On Mon, Feb 24, 2020 at 06:02:17PM +0000, Bae, Chang Seok wrote:
>
>> On Dec 4, 2019, at 12:20, Andy Lutomirski <luto@kernel.org> wrote:
>>
>> I think it could make sense to add a whole new ptrace() command to
>> tell the tracee to, in effect, MOV a specified value to a segment
>> register.  This call would have the actual correct semantics in which
>> it would return an error code if the specified value is invalid and
>> would return 0 on success.  And then a second ptrace() call could be
>> issued to read out FSBASE or GSBASE if needed.  Would this be useful?
>> What gdb commands would invoke it?
>
>We consider new commands to access GDT/LDT that hpa posted before [1] may be
>helpful. If the kernel provides the interfaces to ptracer, we expect GDB for
>both 32-/64-bits can make such changes for inferior calls:
>(1) When FS/GS selector only updated,
>	GDB used to write the selector value via SETREGS. Now it can read the
>	base value from the new APIs and write the base also. This change does
>	not harm today's kernel, and it retains the legacy behavior on
>	FSGSBASE-enabled kernels in the future.
>(2) When FS/GS base only updated,
>(3) When both FS/GS selector and base updated,
>	GDB has no change from what it used to do. The new FSGSBASE-enabled
>	kernel improves the behavior by keeping the base regardless of a
>	selector.
>
>The proposed change in GDB would do an additional GETREGS for every SETREGS
>to obtain the old value. Other ptrace-users may need a similar patch if
>sensitive to the outcome from writing FS/GS selector, but last time when we
>surveyed for other tools [2, 3], we didn't find the issue. We also didn't
>find actual users who rely on legacy behavior in practice.
>
>We'd like to hear a clear opinion of whether the GDB changes along with the
>new ptrace APIs are necessary and sufficient as preparing the FSGSBASE
>support in the kernel.

Hi folks,

Let me try to revive this work as I think that it's blocked due to
misunderstanding of the current situation.

What I gather from the Intel folks is that the GDB folks are okay with
the change as is and don't expect to be doing any changes on their end.

The intel folks are interested in resolving this, but haven't heard back
on their proposed plan (above).

Thomas/Andy want to make sure that we are doing the right thing and are
not breaking anything:

   1. The ptrace modifications are correct (we do the right thing around
   updating FS/GS).
   2. The ptrace changes don't break existing userspace. I think that
   the Intel folks confirmed it above.

Is my attempt at understanding the current situation correct?

-- 
Thanks,
Sasha