From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9B5A7C4332F for ; Fri, 9 Dec 2022 17:04:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3B8C68E0003; Fri, 9 Dec 2022 12:04:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3691E8E0001; Fri, 9 Dec 2022 12:04:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 20A3D8E0003; Fri, 9 Dec 2022 12:04:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 0FBB98E0001 for ; Fri, 9 Dec 2022 12:04:53 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id DF89A1C6D21 for ; Fri, 9 Dec 2022 17:04:52 +0000 (UTC) X-FDA: 80223392424.11.9414638 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf23.hostedemail.com (Postfix) with ESMTP id 534A6140007 for ; Fri, 9 Dec 2022 17:04:49 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ICmW33ah; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf23.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670605489; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=wNfQ2oi3WbWAtZJJ0l7HtHKArD5g54tNYexfqe6B43g=; b=4Gh3sLcaZ2DVcZfXYKtaElQ8j82jYOX9mrLNv98Vq81xSMOhwGF8j+TvY/iP9dqNOkhNHp rnTxw2dnE16Jr7SQuE22DAQZbWqF7UDGQKj6UhOd26zyzra7+eVpgwpsotIZkNHD7jpQhW iV92uss5DV6j1igrOE5prNqiYQ2H/R0= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ICmW33ah; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf23.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670605489; a=rsa-sha256; cv=none; b=bmyYpIuoa1KxHTZm88iz5cZzMBN8/9JqeRhJ7jVlwfjcnLjLUQl3r3lkHEDoxBPoGWdpaT aru+HVwfiY6xjyOud5kRDDqJREsjb959D4E1lhUKyQvTWDwtEBJyX/4/crv9jUqqtU/gfj IfgkAerCTs2BDokkMHYOkitFwpIRFYI= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4D98761E41; Fri, 9 Dec 2022 17:04:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9986C433EF; Fri, 9 Dec 2022 17:04:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670605487; bh=gQXA+j2g4oXs6RnRNrCZ0AkHANYxi91gTp0MorfjTGI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ICmW33ahP9H3MfZN7R7k0WrqPoeMU3QossaBBbInxc9p29nNkyqIKVKZKPN3wvnMA H4gGLoeNiu/SJ4nUCF+01yi89YfqqGZmaH4ykfsg7Zakdisjl7y0+YWjRRsv09cwSZ ysEMrqXZRcUCLdgxLP/y9QD0LU+r4mCFQnC1kslVdO6ok3RSwflZiTSquCHlvQyAdZ 2K1K7BBbqbV01VjBT8Rt2u9ffacu/fJ46WI51h0DhQfQt4/fmI+/QixriXEana3qRj pa2KH0kR3m8xVYpjPl/j8R5voWGbnqrpqQTUyBaQPS7no8zEIEskmkPOSSlIWC+l1y L5UxNWbAnZrYQ== Date: Fri, 9 Dec 2022 19:04:24 +0200 From: Mike Rapoport To: Rick Edgecombe Cc: x86@kernel.org, "H . Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Borislav Petkov , Cyrill Gorcunov , Dave Hansen , Eugene Syromiatnikov , Florian Weimer , "H . J . Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , Randy Dunlap , Weijiang Yang , "Kirill A . Shutemov" , John Allen , kcc@google.com, eranian@google.com, jamorris@linux.microsoft.com, dethoma@microsoft.com, akpm@linux-foundation.org, Andrew.Cooper3@citrix.com, christina.schimpe@intel.com, Yu-cheng Yu Subject: Re: [PATCH v4 37/39] x86: Add PTRACE interface for shadow stack Message-ID: References: <20221203003606.6838-1-rick.p.edgecombe@intel.com> <20221203003606.6838-38-rick.p.edgecombe@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221203003606.6838-38-rick.p.edgecombe@intel.com> X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 534A6140007 X-Stat-Signature: 7zhdruaejs5foahwkbkcgpw8jxhn5d9c X-HE-Tag: 1670605489-641163 X-HE-Meta: U2FsdGVkX18hDcazcbXQQMg13Ge+CmhBbcmtZ3mLhTQAHyPIjIEYQjIaOKTcqsb6kEsq4LPnGoYuyd8F65M4MCpxjbMGNYyMFnID49+3Y1RFVuC4iv9VZoapAYQ8aIA2bfhVLeKDnyvsw06erXsOMzT7mXSJ3DjbNZBsvm18H6WQ5MSfwbPLZ9/T9hK0fvs3+k1/h8wcCnTwu1B6YOr9JW/TYUvcqk+9n04g3UNk5EM/1yiUeg5htL5S6gNUL8WA+26QNOZEx9uuIwG3/RW2cKvGS/uIrWkMIjbVxxsX3K2cGsqFTqBXAU8TzIfjQCue9khGDVH9qEP7xW4UmGKRB8LJ30QPJTggzEVQAR24exa6MM737+kQz8eRkpNWDdm68HFagMY7Fs/ZpLhjpsJRHjngTAmG3YQpM0wbS5IHkDqsYhs57QUcP4bd/jicX8cfxnrzrV1r35rG0+TfJv+NVwO35v9ezJDNblyaPI/5UAPYVQTPYqf9jCaDlodyXP4feETDG/s3NikB/TXETtRpUOXRLBUhlYroFGQEKfczgA1Ar23DRvPt4PwL1zMMvUGGnjqWlzs2ppW21dEJQzdfsBaPAR13TjnahhCSMTONrg3GyrRIVrH1UmR5srV+cel4tnTFmMi2ha/ZtpMa9VodvljUJ7Gr652WBxqEVa+eF/ZDoOU2IwaPEYhFsiNrGWlHgvhel0pkeP2iC0ubv8Fn0NIvX+CkzgaP8nF1QInXlhi8pMd8j9lMEhO8uEJSysiYyvznvEqfPvVhxO7YylFhh0SbVQQ8X7OFmTci56bXC9+40A7HaPFCqVJQnz2/pj0IUc+yxFKV663GZECwmvbjPGm2pPV1Hqczrclea+9pfdG9XLRhbGscPKqx2K6pnRP2+0IHKdCkTDtHYYqVqQhzxvKx7WWJxegiSkR+gQOm19BSbGKdeEMwbXzTXeaQiUn5jSz3yYUoylIOBfrV36P z34SlbbB rjOgi7/fH8bWRPFNfcSr4Aa9knTl3qHgg5AiffTP/ZNnMlyT3fXkbVfWcyMbieaf8jnCPuSAEGghB3p75jey4eIfKGao3YjDiflqjYYeqnrBlwCM= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Rick, On Fri, Dec 02, 2022 at 04:36:04PM -0800, Rick Edgecombe wrote: > From: Yu-cheng Yu > > Some applications (like GDB) would like to tweak shadow stack state via > ptrace. This allows for existing functionality to continue to work for > seized shadow stack applications. Provide an regset interface for > manipulating the shadow stack pointer (SSP). > > There is already ptrace functionality for accessing xstate, but this > does not include supervisor xfeatures. So there is not a completely > clear place for where to put the shadow stack state. Adding it to the > user xfeatures regset would complicate that code, as it currently shares > logic with signals which should not have supervisor features. > > Don't add a general supervisor xfeature regset like the user one, > because it is better to maintain flexibility for other supervisor > xfeatures to define their own interface. For example, an xfeature may > decide not to expose all of it's state to userspace, as is actually the > case for shadow stack ptrace functionality. A lot of enum values remain > to be used, so just put it in dedicated shadow stack regset. > > The only downside to not having a generic supervisor xfeature regset, > is that apps need to be enlightened of any new supervisor xfeature > exposed this way (i.e. they can't try to have generic save/restore > logic). But maybe that is a good thing, because they have to think > through each new xfeature instead of encountering issues when new a new > supervisor xfeature was added. > > By adding a shadow stack regset, it also has the effect of including the > shadow stack state in a core dump, which could be useful for debugging. > > The shadow stack specific xstate includes the SSP, and the shadow stack > and WRSS enablement status. Enabling shadow stack or wrss in the kernel > involves more than just flipping the bit. The kernel is made aware that > it has to do extra things when cloning or handling signals. That logic > is triggered off of separate feature enablement state kept in the task > struct. So the flipping on HW shadow stack enforcement without notifying > the kernel to change its behavior would severely limit what an application > could do without crashing, and the results would depend on kernel > internal implementation details. There is also no known use for controlling > this state via prtace today. So only expose the SSP, which is something > that userspace already has indirect control over. > > Tested-by: Pengfei Xu > Tested-by: John Allen > Co-developed-by: Rick Edgecombe > Signed-off-by: Rick Edgecombe > Signed-off-by: Yu-cheng Yu > --- > > v4: > - Make shadow stack only. Reduce to only supporting SSP register, and > remove CET references (peterz) > - Add comment to not use 0x203, becuase binutils already looks for it in > coredumps. (Christina Schimpe) > > v3: > - Drop dependence on thread.shstk.size, and use thread.features bits > - Drop 32 bit support > > v2: > - Check alignment on ssp. > - Block IBT bits. > - Handle init states instead of returning error. > - Add verbose commit log justifying the design. > > Yu-Cheng v12: > - Return -ENODEV when CET registers are in INIT state. > - Check reserved/non-support bits from user input. > > arch/x86/include/asm/fpu/regset.h | 7 +-- > arch/x86/kernel/fpu/regset.c | 87 +++++++++++++++++++++++++++++++ > arch/x86/kernel/ptrace.c | 12 +++++ > include/uapi/linux/elf.h | 2 + > 4 files changed, 105 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/include/asm/fpu/regset.h b/arch/x86/include/asm/fpu/regset.h > index 4f928d6a367b..697b77e96025 100644 > --- a/arch/x86/include/asm/fpu/regset.h > +++ b/arch/x86/include/asm/fpu/regset.h > @@ -7,11 +7,12 @@ > > #include > > -extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active; > +extern user_regset_active_fn regset_fpregs_active, regset_xregset_fpregs_active, > + ssp_active; > extern user_regset_get2_fn fpregs_get, xfpregs_get, fpregs_soft_get, > - xstateregs_get; > + xstateregs_get, ssp_get; > extern user_regset_set_fn fpregs_set, xfpregs_set, fpregs_soft_set, > - xstateregs_set; > + xstateregs_set, ssp_set; > > /* > * xstateregs_active == regset_fpregs_active. Please refer to the comment > diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c > index 6d056b68f4ed..00f3d5c9b682 100644 > --- a/arch/x86/kernel/fpu/regset.c > +++ b/arch/x86/kernel/fpu/regset.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > > #include "context.h" > #include "internal.h" > @@ -174,6 +175,92 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, > return ret; > } > > + > +#ifdef CONFIG_X86_USER_SHADOW_STACK > +int ssp_active(struct task_struct *target, const struct user_regset *regset) > +{ > + if (shstk_enabled()) This is not going to work with ptrace as shstk_enabled() checks current rather than target. > + return regset->n; > + > + return 0; > +} > + > +int ssp_get(struct task_struct *target, const struct user_regset *regset, > + struct membuf to) > +{ > + struct fpu *fpu = &target->thread.fpu; > + struct cet_user_state *cetregs; > + > + if (!boot_cpu_has(X86_FEATURE_USER_SHSTK)) > + return -ENODEV; > + > + sync_fpstate(fpu); > + cetregs = get_xsave_addr(&fpu->fpstate->regs.xsave, XFEATURE_CET_USER); > + if (!cetregs) { > + /* > + * The registers are the in the init state. The init values for > + * these regs are zero, so just zero the output buffer. > + */ > + membuf_zero(&to, sizeof(cetregs->user_ssp)); > + return 0; > + } > + > + return membuf_write(&to, (unsigned long *)&cetregs->user_ssp, > + sizeof(cetregs->user_ssp)); > +} > + > +int ssp_set(struct task_struct *target, const struct user_regset *regset, > + unsigned int pos, unsigned int count, > + const void *kbuf, const void __user *ubuf) > +{ > + struct fpu *fpu = &target->thread.fpu; > + struct xregs_state *xsave = &fpu->fpstate->regs.xsave; > + struct cet_user_state *cetregs; > + unsigned long user_ssp; > + int r; > + > + if (!boot_cpu_has(X86_FEATURE_USER_SHSTK) || > + !ssp_active(target, regset)) > + return -ENODEV; > + > + r = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &user_ssp, 0, -1); > + if (r) > + return r; > + > + /* > + * Some kernel instructions (IRET, etc) can cause exceptions in the case > + * of disallowed CET register values. Just prevent invalid values. > + */ > + if ((user_ssp >= TASK_SIZE_MAX) || !IS_ALIGNED(user_ssp, 8)) > + return -EINVAL; > + > + fpu_force_restore(fpu); > + > + /* > + * Don't want to init the xfeature until the kernel will definetely > + * overwrite it, otherwise if it inits and then fails out, it would > + * end up initing it to random data. > + */ > + if (!xfeature_saved(xsave, XFEATURE_CET_USER) && > + WARN_ON(init_xfeature(xsave, XFEATURE_CET_USER))) > + return -ENODEV; > + > + cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER); > + if (WARN_ON(!cetregs)) { > + /* > + * This shouldn't ever be NULL because it was successfully > + * inited above if needed. The only scenario would be if an > + * xfeature was somehow saved in a buffer, but not enabled in > + * xsave. > + */ > + return -ENODEV; > + } > + > + cetregs->user_ssp = user_ssp; > + return 0; > +} > +#endif /* CONFIG_X86_USER_SHADOW_STACK */ > + > #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION > > /* > diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c > index dfaa270a7cc9..095f04bdabdc 100644 > --- a/arch/x86/kernel/ptrace.c > +++ b/arch/x86/kernel/ptrace.c > @@ -58,6 +58,7 @@ enum x86_regset_64 { > REGSET64_FP, > REGSET64_IOPERM, > REGSET64_XSTATE, > + REGSET64_SSP, > }; > > #define REGSET_GENERAL \ > @@ -1267,6 +1268,17 @@ static struct user_regset x86_64_regsets[] __ro_after_init = { > .active = ioperm_active, > .regset_get = ioperm_get > }, > +#ifdef CONFIG_X86_USER_SHADOW_STACK > + [REGSET64_SSP] = { > + .core_note_type = NT_X86_SHSTK, > + .n = 1, > + .size = sizeof(u64), > + .align = sizeof(u64), > + .active = ssp_active, > + .regset_get = ssp_get, > + .set = ssp_set > + }, > +#endif > }; > > static const struct user_regset_view user_x86_64_view = { > diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h > index c7b056af9ef0..e9283f0641c4 100644 > --- a/include/uapi/linux/elf.h > +++ b/include/uapi/linux/elf.h > @@ -406,6 +406,8 @@ typedef struct elf64_shdr { > #define NT_386_TLS 0x200 /* i386 TLS slots (struct user_desc) */ > #define NT_386_IOPERM 0x201 /* x86 io permission bitmap (1=deny) */ > #define NT_X86_XSTATE 0x202 /* x86 extended state using xsave */ > +/* Old binutils treats 0x203 as a CET state */ > +#define NT_X86_SHSTK 0x204 /* x86 SHSTK state */ > #define NT_S390_HIGH_GPRS 0x300 /* s390 upper register halves */ > #define NT_S390_TIMER 0x301 /* s390 timer register */ > #define NT_S390_TODCMP 0x302 /* s390 TOD clock comparator register */ > -- > 2.17.1 > -- Sincerely yours, Mike.