linux-kernel - RE: [PATCH] x86: Check if shadow stack is active for ssp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR11MB763665BE84DE23DCC24BAFA1F9112@PH0PR11MB7636.namprd11.prod.outlook.com>
Date: Tue, 7 Jan 2025 10:18:06 +0000
From: "Schimpe, Christina" <christina.schimpe@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "x86@...nel.org"
	<x86@...nel.org>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "hpa@...or.com"
	<hpa@...or.com>, "Lutomirski, Andy" <luto@...nel.org>, "peterz@...radead.org"
	<peterz@...radead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: "Metzger, Markus T" <markus.t.metzger@...el.com>
Subject: RE: [PATCH] x86: 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 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@...el.com>
> Tested-by: Christina Schimpe <christina.schimpe@...el.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> ---
>  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
> 6bc1eb2a21bd..887b0b8e21e3 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);
> --
> 2.34.1

gdb uses this ptrace call to read the shadow stack register:
https://sourceware.org/pipermail/gdb-patches/2024-December/214322.html

We see the warning regularly when running the testsuite.
Therefore, it would be great if we could merge this patch.

Kind Regards
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928