The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 9b9ab249c4b1babb0d36c9d4f3309cfb6901a15a
Gitweb: https://git.kernel.org/tip/9b9ab249c4b1babb0d36c9d4f3309cfb6901a15a
Author: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
AuthorDate: Mon, 04 Dec 2023 11:07:09 -08:00
Committer: Borislav Petkov (AMD) <bp@xxxxxxxxx>
CommitterDate: Tue, 07 Jan 2025 12:08:40 +01:00
x86/fpu: Check if shadow stack is active for ssp_get()
The shadow stack regset ->set() handler (ssp_set()) checks the regset
->active() handler (ssp_active()) to verify that shadow stack is active. When
shadow stack is active, the XFEATURE_CET_USER xfeature will not be in the init
state because there is at least one bit set (SHSTK_EN). So ssp_set() should be
able to safely operate on the xfeature in the xsave buffer after checking
ssp_active(). If it finds it is in the init state anyway, it warns because an
unexpected situation has been encountered.
But ssp_get(), the regset_get() handler, doesn't check ssp_active(). This
was under the assumption that all the callers check the ->active()
handler. It is indeed normally the case, but Christina Schimpe reports that
and a warning like the following can be generated:
WARNING: CPU: 5 PID: 1773 at arch/x86/kernel/fpu/regset.c:198 ssp_get+0x89/0xa0
[...]
Call Trace:
<TASK>
? show_regs+0x6e/0x80
? ssp_get+0x89/0xa0
? __warn+0x91/0x150
? ssp_get+0x89/0xa0
? report_bug+0x19d/0x1b0
? handle_bug+0x46/0x80
? exc_invalid_op+0x1d/0x80
? asm_exc_invalid_op+0x1f/0x30
? __pfx_ssp_get+0x10/0x10
? ssp_get+0x89/0xa0
? ssp_get+0x52/0xa0
__regset_get+0xad/0xf0
copy_regset_to_user+0x52/0xc0
ptrace_regset+0x119/0x140
ptrace_request+0x13c/0x850
? wait_task_inactive+0x142/0x1d0
? do_syscall_64+0x6d/0x90
arch_ptrace+0x102/0x300
[...]
It turns out the PTRACE_GETREGSET path does not check ssp_active(). The issue
could be fixed by just removing the warning, but it would be nicer to rely
on a check of ssp_active() which is much easier to reason about than xsave
init state logic. So add a ssp_active() check in ssp_get() like there already
is in ssp_set().
Fixes: 2fab02b25ae7 ("x86: Add PTRACE interface for shadow stack")
Reported-by: Christina Schimpe <christina.schimpe@xxxxxxxxx>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
Tested-by: Christina Schimpe <christina.schimpe@xxxxxxxxx>
Cc: <stable@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20231204190709.3907254-1-rick.p.edgecombe@xxxxxxxxx
---
arch/x86/kernel/fpu/regset.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 6bc1eb2..887b0b8 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -190,7 +190,8 @@ int ssp_get(struct task_struct *target, const struct user_regset *regset,
struct fpu *fpu = &target->thread.fpu;
struct cet_user_state *cetregs;
- if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK))
+ if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) ||
+ !ssp_active(target, regset))
return -ENODEV;
sync_fpstate(fpu);
|