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 3746CCFD37C for ; Fri, 11 Oct 2024 12:33:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 932FC6B0088; Fri, 11 Oct 2024 08:33:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8E2126B00A8; Fri, 11 Oct 2024 08:33:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 75A946B00AA; Fri, 11 Oct 2024 08:33:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 574A36B0088 for ; Fri, 11 Oct 2024 08:33:12 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 2446EACABB for ; Fri, 11 Oct 2024 12:33:03 +0000 (UTC) X-FDA: 82661261340.17.C228AF6 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf18.hostedemail.com (Postfix) with ESMTP id 6E2581C0019 for ; Fri, 11 Oct 2024 12:33:09 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=XBuRTnFB; spf=pass (imf18.hostedemail.com: domain of broonie@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=broonie@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728649852; 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=G9H0zTGlDxBoyk3O6Fng7yaGddd6GE+lemaia6MSF8c=; b=GrVrGQyScW5Q1+9B3JFzEH1EGVu8Q6y12dLa2FLLR+1tyPnG2PEKMPCewULfC404CeeO6X Czby29DhiX35hE1Wbd6q3aDX42ErpU281kbEyg8iwTq7KeQLccHxjUB07VOoRMLzlop5nM 4znahdeJRZBqYsMx2S/zEUGQeNidAcU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728649852; a=rsa-sha256; cv=none; b=14SpIKNzVecX0BCJo93GCwNEThgEFzW9bkTMxk2ZIhrptho6X4DZv+Z5F/tftRBcvI6EiO BdQzlt7Nq5bFk6ymc305TVZwtliyQkM9BS8AA55mrP1bw5AJuAUoKvP+Ds+SAGzDCoFnj+ KznD495igYcihe7DJajaOKjrTU0FDEM= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=XBuRTnFB; spf=pass (imf18.hostedemail.com: domain of broonie@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=broonie@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id AFBE95C5BEC; Fri, 11 Oct 2024 12:33:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D14D3C4CEC3; Fri, 11 Oct 2024 12:33:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1728649988; bh=6WlLSqQl5UgDYGtPwPNUpV33/lVuWnuZFTN12+K3zSQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XBuRTnFB7tRS+B52FfbK+aqz5vqrJKQLgUY6D1lgVXevJiQ9aOQiQ7jw7p8TXy9dY a1JCpat1nH+oXyqCfZw1iSqjWRr4ISWRY/YY+0oEBhSxGWvQYLYaUuGPxiw0p25Nfq +KL1EeZV2hifOBDc5TXTne7ReQ4gx8SyOoII2RaQoDc2guXhjhuWJ9+vcnTT4y02na AndbSZ5lS+HRdx00pBh8feOWAF4CjGXByFgEPFTAjNWvxGztY5t7Q3p1RCRpXV6V49 yT4xjlGPyPucYd8zjCAX7+3OB9kLw7PC0p74wr9yLfcqjG+un85hRpgeHMWRU5vjsv +4CMr6EyOHNFw== Date: Fri, 11 Oct 2024 13:33:05 +0100 From: Mark Brown To: Deepak Gupta Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Andrew Morton , "Liam R. Howlett" , Vlastimil Babka , Lorenzo Stoakes , Arnd Bergmann , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, Rick Edgecombe Subject: Re: [PATCH RFC/RFT 3/3] kernel: converge common shadow stack flow agnostic to arch Message-ID: References: <20241010-shstk_converge-v1-0-631beca676e7@rivosinc.com> <20241010-shstk_converge-v1-3-631beca676e7@rivosinc.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="SJuq1KBFwUpX1bg5" Content-Disposition: inline In-Reply-To: <20241010-shstk_converge-v1-3-631beca676e7@rivosinc.com> X-Cookie: Editing is a rewording activity. X-Rspamd-Queue-Id: 6E2581C0019 X-Stat-Signature: ayh6tyb1iucqxa8buk8ab6e45t3z3jea X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1728649989-333440 X-HE-Meta: U2FsdGVkX188xiKZXv408Kqs3ecTRoT7qzDEj8VkWxcceBQ8yjAtIT6A6nhTcjVslKJVJkDLZs6R4nEDAZglB5K7oQleBNOh0ZLdVaRlqln4yOi5erj1ZC7mygF507p44yBdMo6LAvd/CszR/1jD4Kq6mAvOVj5nrXNa992VpOcQfKTKd2ONIHf11b7gX65JEejIswQUbte0ZNWsE4kU8ojOtkNDH7qxVv6/F3+BhwxDGD4K6k5qvyP39AdN9jpdxgqRwICBFjFUsrVVuQUCWzpY9ooIdW/JDBNrPyA8Vj1Y55f/L6u/OsbVS3phYmA3kTtzEx9defDIJ/LraQpLZt6A9/2RwBrA6BOG0vVCVDtbJzAOzd3NFELRQldjXKfNb2buzflilAH8WUxmvy4Y6pe1LRp9+ZnoyfHNALJe20yVAbC3tQl9rWkhwVoyngYvOdRjeHGkkr3ikdJLzb5D+FCK7uZsWsYOV3uJRtCRqo1V3Dn6cxP8ruFRxhVT+iKvGskrSyE2iH9lvFSev2HG7dvipNMyVF9ySU4rODvhxffmsOyZXi3wy44zMw1KpvLPes0xwBcd+jsiQeXznefmGH8OoCfC5rG+HUmZf76lOlmb/QOzleqBQl7xotxrvnu0kWWKkabFKCHSOMxypt+wkGbzawAulN098OvtJUSama4AW/IPyJecpuT8mHZ+P9mPOjX7Pqd9aLe/T4pPlIWO3eNOHf5a3e/WX3tA2sX4UxQYzhc9JX6AKG7Kxilm0a0nGijU1Mq848vfdRgvwe9Vsx8lNmM0nt2qFmaoqhr7Rdibhifl5z1uKEijZbbH/Ex6zketrpCvi4vLoOdLAp28grAlAWVMMrStbiufKj87tI+NaDefKUUh01jStlydsrtTkqvz8NjcD4FxcTabBfftSqBiHqsYmZjK/V/3xvWkm7AzMMimTLjvp0a8nXhqmgbNCKGYseWW5GnA4lLoPKS BYsQxlfU XfFGrzJ/DwntglBJ5gjBjmhHq1Tezu23O6bp1VA2MoZynCpURrcJC3aCqPvWWu7KgxtmQmAlqMFCi/nmqZIRGcsOmOEpTaIPcaAMXh3GL/tYiuYVc9TvDeApCLBGOhoJw8/7DiT4uZGs/fabL0cnmf4HuGkk0vqQoQD2ILoaYGxSbZE/f5tcESslf/wonDnNhMqoEivlWjGyZabIHkjFNU6XjG33JDQ7OJANSUt0j/Y8sDYeO2zzzkqLMUTr98nJp+WmODHdTJP5ZEODFWgb70QOJDvOUTfoTJ8ofoNfYHTXO6tSNYjVK4Wjf+rYXUYCiY9hg 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: List-Subscribe: List-Unsubscribe: --SJuq1KBFwUpX1bg5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 10, 2024 at 05:32:05PM -0700, Deepak Gupta wrote: > +unsigned long alloc_shstk(unsigned long addr, unsigned long size, > + unsigned long token_offset, bool set_res_tok); > +int shstk_setup(void); > +int create_rstor_token(unsigned long ssp, unsigned long *token_addr); > +bool cpu_supports_shadow_stack(void); The cpu_ naming is confusing in an arm64 context, we use cpu_ for functions that report if a feature is supported on the current CPU and system_ for functions that report if a feature is enabled on the system. > +void set_thread_shstk_status(bool enable); It might be better if this took the flags that the prctl() takes? It feels like=20 > +/* Flags for map_shadow_stack(2) */ > +#define SHADOW_STACK_SET_TOKEN (1ULL << 0) /* Set up a restore token in = the shadow stack */ > + We've also got SHADOW_STACK_SET_MARKER now. > +bool cpu_supports_shadow_stack(void) > +{ > + return arch_cpu_supports_shadow_stack(); > +} > + > +bool is_shstk_enabled(struct task_struct *task) > +{ > + return arch_is_shstk_enabled(task); > +} Do we need these wrappers (or could they just be static inlines in the header)? > +void set_thread_shstk_status(bool enable) > +{ > + arch_set_thread_shstk_status(enable); > +} arm64 can return an error here, we reject a bunch of conditions like 32 bit threads and locked enable status. > +unsigned long adjust_shstk_size(unsigned long size) > +{ > + if (size) > + return PAGE_ALIGN(size); > + > + return PAGE_ALIGN(min_t(unsigned long long, rlimit(RLIMIT_STACK), SZ_4G= )); > +} static? > +/* > + * VM_SHADOW_STACK will have a guard page. This helps userspace protect > + * itself from attacks. The reasoning is as follows: > + * > + * The shadow stack pointer(SSP) is moved by CALL, RET, and INCSSPQ. The > + * INCSSP instruction can increment the shadow stack pointer. It is the > + * shadow stack analog of an instruction like: > + * > + * addq $0x80, %rsp > + * > + * However, there is one important difference between an ADD on %rsp > + * and INCSSP. In addition to modifying SSP, INCSSP also reads from the > + * memory of the first and last elements that were "popped". It can be > + * thought of as acting like this: > + * > + * READ_ONCE(ssp); // read+discard top element on stack > + * ssp +=3D nr_to_pop * 8; // move the shadow stack > + * READ_ONCE(ssp-8); // read+discard last popped stack element > + * > + * The maximum distance INCSSP can move the SSP is 2040 bytes, before > + * it would read the memory. Therefore a single page gap will be enough > + * to prevent any operation from shifting the SSP to an adjacent stack, > + * since it would have to land in the gap at least once, causing a > + * fault. This is all very x86 centric... > + if (create_rstor_token(mapped_addr + token_offset, NULL)) { > + vm_munmap(mapped_addr, size); > + return -EINVAL; > + } Bikeshedding but can we call the function create_shstk_token() instead? The rstor means absolutely nothing in an arm64 context. > +SYSCALL_DEFINE3(map_shadow_stack, unsigned long, addr, unsigned long, si= ze, unsigned int, flags) > +{ > + bool set_tok =3D flags & SHADOW_STACK_SET_TOKEN; > + unsigned long aligned_size; > + > + if (!cpu_supports_shadow_stack()) > + return -EOPNOTSUPP; > + > + if (flags & ~SHADOW_STACK_SET_TOKEN) > + return -EINVAL; This needs SHADOW_STACK_SET_MARKER for arm64. > + if (addr && (addr & (PAGE_SIZE - 1))) > + return -EINVAL; if (!PAGE_ALIGNED(addr)) > +int shstk_setup(void) > +{ This is half of the implementation of the prctl() for enabling shadow stacks. Looking at the arm64 implementation this rafactoring feels a bit awkward, we don't have the one flag at a time requiremet that x86 has and we structure things rather differently. I'm not sure that the arch_prctl() and prctl() are going to line up comfortably... > + struct thread_shstk *shstk =3D ¤t->thread.shstk; > + unsigned long addr, size; > + > + /* Already enabled */ > + if (is_shstk_enabled(current)) > + return 0; > + > + /* Also not supported for 32 bit */ > + if (!cpu_supports_shadow_stack() || > + (IS_ENABLED(CONFIG_X86_64) && in_ia32_syscall())) > + return -EOPNOTSUPP; We probably need a thread_supports_shstk(), arm64 has a similar check for not 32 bit threads and I noted an issue with needing this check elsewhere. > + /* > + * For CLONE_VFORK the child will share the parents shadow stack. > + * Make sure to clear the internal tracking of the thread shadow > + * stack so the freeing logic run for child knows to leave it alone. > + */ > + if (clone_flags & CLONE_VFORK) { > + set_shstk_base_size(tsk, 0, 0); > + return 0; > + } On arm64 we set the new thread's shadow stack pointer here, the logic around that can probably also be usefully factored out. > + /* > + * For !CLONE_VM the child will use a copy of the parents shadow > + * stack. > + */ > + if (!(clone_flags & CLONE_VM)) > + return 0; Here also. --SJuq1KBFwUpX1bg5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmcJGwAACgkQJNaLcl1U h9CbtAf8DH3tQWEmB0fJgCHXSmvsgdU8Z/4z+OUKw5cYTHGo1EsEBpfrOQCevbwL LFr5TnnmBv5ZeYYXdjvi/jrtNJot6dN3s2PMHNoVk1HmIwWRKAnkqcwKnw2XJ2IY 65RBT1iAQHHPMw6IV5l6J9ipayRIXPrjuFQG+n9tj9CEprTMhRE13FpjagOk3Ncl xpoyKXjjxJqu2rI3VKBoMnzjzs/5El7ru0de+ALqZOUvQDKdVt3fGERJQm0TrSeG Y1rPvBCbqg6+J8+xIsPS7Ba/QJfgC1ZUj2eYEAJyVOJvwQ/C9NKHokV/vYlQZ1uD 0zR4iOT0Xj19R0+4TrBaAwdtbmXOgQ== =z0Cu -----END PGP SIGNATURE----- --SJuq1KBFwUpX1bg5--