Debuggers expect that doing PTRACE_GETREGS, then poking at a tracee
and maybe letting it run for a while, then doing PTRACE_SETREGS will
put the tracee back where it was. In the specific case of a 32-bit
tracer and tracee, the PTRACE_GETREGS/SETREGS data structure doesn't
have fs_base or gs_base fields, so FSBASE and GSBASE fields are
never stored anywhere. Everything used to still work because
nonzero FS or GS would result full reloads of the segment registers
when the tracee resumes, and the bases associated with FS==0 or
GS==0 are irrelevant to 32-bit code.
Adding FSGSBASE support broke this: when FSGSBASE is enabled, FSBASE
and GSBASE are now restored independently of FS and GS for all tasks
when context-switched in. This means that, if a 32-bit tracer
restores a previous state using PTRACE_SETREGS but the tracee's
pre-restore and post-restore bases don't match, then the tracee is
resumed with the wrong base.
Fix it by explicitly loading the base when a 32-bit tracer pokes FS
or GS on a 64-bit kernel.
Also add a test case.
Fixes: 673903495c85 ("x86/process/64: Use FSBSBASE in switch_to() if available")
Cc: Sasha Levin
Signed-off-by: Andy Lutomirski
---
arch/x86/include/asm/fsgsbase.h | 2 +
arch/x86/kernel/process_64.c | 4 +-
arch/x86/kernel/ptrace.c | 43 ++-
tools/testing/selftests/x86/Makefile | 2 +-
.../testing/selftests/x86/fsgsbase_restore.c | 245 ++
5 files changed, 280 insertions(+), 16 deletions(-)
create mode 100644 tools/testing/selftests/x86/fsgsbase_restore.c
diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h
index aefd53767a5d..d552646411a9 100644
--- a/arch/x86/include/asm/fsgsbase.h
+++ b/arch/x86/include/asm/fsgsbase.h
@@ -75,6 +75,8 @@ static inline void x86_fsbase_write_cpu(unsigned long fsbase)
extern unsigned long x86_gsbase_read_cpu_inactive(void);
extern void x86_gsbase_write_cpu_inactive(unsigned long gsbase);
+extern unsigned long x86_fsgsbase_read_task(struct task_struct *task,
+ unsigned short selector);
#endif /* CONFIG_X86_64 */
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index d618969cea66..cb8e37d3acaa 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -347,8 +347,8 @@ static __always_inline void x86_fsgsbase_load(struct
thread_struct *prev,
}
}
-static unsigned long x86_fsgsbase_read_task(struct task_struct *task,
- unsigned short selector)
+unsigned long x86_fsgsbase_read_task(struct task_struct *task,
+unsigned short selector)
{
unsigned short idx = selector >> 3;
unsigned long base;
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 1c7646c8d0fe..3f006489087f 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -281,17 +281,9 @@ static int set_segment_reg(struct task_struct *task,
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.
+* Writes to FS and GS will change the stored selector. Whether
+* this changes the segment base as well depends on whether
+* FSGSBASE is enabled.
*/
switch (offset) {
@@ -867,14 +859,39 @@ long arch_ptrace(struct task_struct *child, long request,
static int putreg32(struct task_struct *child, unsigned regno, u32 value)
{
struct pt_regs *regs = task_pt_regs(child);
+ int ret;
switch (regno) {
SEG32(cs);
SEG32(ds);
SEG32(es);
- SEG32(fs);
- SEG32(gs);
+
+ /*
+* A 32-bit ptracer on a 64-bit kernel expects that writing
+* FS or GS will also update the base. This is needed for
+* operations like PTRACE_SETREGS to fully restore a saved
+* CPU state.
+*/
+
+ case offsetof(struct user32, regs.fs):
+ ret = set_segment_reg(child,
+ offsetof(struct user_regs_struct, fs),
+ value);
+ if (ret == 0)
+ child->thread.fsbase =
+ x86_fsgsbase_read_task(child, value);
+ return ret;
+
+ case offsetof(struct user32, regs.gs):
+
