[tip: x86/urgent] x86/ptrace: Document FSBASE and GSBASE ABI oddities

From: tip-bot2 for Andy Lutomirski
Date: Wed Nov 27 2019 - 03:20:07 EST


The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 56f2ab41b652251f336a0f471b1033afeaedd161
Gitweb: https://git.kernel.org/tip/56f2ab41b652251f336a0f471b1033afeaedd161
Author: Andy Lutomirski <luto@xxxxxxxxxx>
AuthorDate: Wed, 17 Jul 2019 06:44:16 -07:00
Committer: Ingo Molnar <mingo@xxxxxxxxxx>
CommitterDate: Tue, 26 Nov 2019 22:00:12 +01:00

x86/ptrace: Document FSBASE and GSBASE ABI oddities

Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
---
arch/x86/kernel/ptrace.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 3b3b169..f0e1ddb 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -281,6 +281,20 @@ static int set_segment_reg(struct task_struct *task,
if (invalid_selector(value))
return -EIO;

+ /*
+ * This function has some ABI oddities.
+ *
+ * A 32-bit ptracer probably expects that writing FS or GS will change
+ * FSBASE or GSBASE respectively. In the absence of FSGSBASE support,
+ * this code indeed has that effect. When FSGSBASE is added, this
+ * will require a special case.
+ *
+ * For existing 64-bit ptracers, writing FS or GS *also* currently
+ * changes the base if the selector is nonzero the next time the task
+ * is run. This behavior may not be needed, and trying to preserve it
+ * when FSGSBASE is added would be complicated at best.
+ */
+
switch (offset) {
case offsetof(struct user_regs_struct,fs):
task->thread.fsindex = value;
@@ -370,6 +384,9 @@ static int putreg(struct task_struct *child,
* When changing the FS base, use do_arch_prctl_64()
* to set the index to zero and to set the base
* as requested.
+ *
+ * NB: This behavior is nonsensical and likely needs to
+ * change when FSGSBASE support is added.
*/
if (child->thread.fsbase != value)
return do_arch_prctl_64(child, ARCH_SET_FS, value);