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 1ED94C3DA7F for ; Thu, 15 Aug 2024 15:33:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8A8AD6B009F; Thu, 15 Aug 2024 11:33:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8312E6B0105; Thu, 15 Aug 2024 11:33:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 683A96B0106; Thu, 15 Aug 2024 11:33:38 -0400 (EDT) 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 43C796B009F for ; Thu, 15 Aug 2024 11:33:38 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id ED07A1C52B5 for ; Thu, 15 Aug 2024 15:33:37 +0000 (UTC) X-FDA: 82454874474.11.E1A75D8 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf09.hostedemail.com (Postfix) with ESMTP id D214414003F for ; Thu, 15 Aug 2024 15:33:35 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf09.hostedemail.com: domain of Dave.Martin@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=Dave.Martin@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723736003; a=rsa-sha256; cv=none; b=GSpsE3IOlCeZT39XOXr0PZV0d3/9Fh9lv3yv3vx8gghHmekX+2+02MvYHKATYePmKSdC2Z lZKiVDkIUo9ig8u+jKcxM0zjO3hfUjkkIQrxxIs+L3VV4pdgtuqZQc1OO90Q+bk0Hpf1AQ SafLJZLWgPUcXycP8PUXL2kQ5y1MMaA= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf09.hostedemail.com: domain of Dave.Martin@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=Dave.Martin@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723736003; 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; bh=thV+icgWgLhrStru+zV1Q9digrGKb6lXMEajA/O80cY=; b=t61jvycf/HZ4wolOcQM7xL7yzGoOBFSzDjktDfC2uiqV2tCLQUCS5IluViMhBaqxKYfwx3 ytr/eyNO2+xlIXtw3+rQN8WwLAqRWz04DS4E4t/AwsohEwpS93hVq9y03C0tB9x4WK2Gkb VLYdlcjDeeHGR7I9GZc+fMWIEV+X8Qg= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7948614BF; Thu, 15 Aug 2024 08:34:00 -0700 (PDT) Received: from e133380.arm.com (e133380.arm.com [10.1.197.55]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 555713F58B; Thu, 15 Aug 2024 08:33:28 -0700 (PDT) Date: Thu, 15 Aug 2024 16:33:25 +0100 From: Dave Martin To: Mark Brown Cc: Catalin Marinas , Will Deacon , Jonathan Corbet , Andrew Morton , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Arnd Bergmann , Oleg Nesterov , Eric Biederman , Shuah Khan , "Rick P. Edgecombe" , Deepak Gupta , Ard Biesheuvel , Szabolcs Nagy , Kees Cook , "H.J. Lu" , Paul Walmsley , Palmer Dabbelt , Albert Ou , Florian Weimer , Christian Brauner , Thiago Jung Bauermann , Ross Burton , linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, kvmarm@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v10 24/40] arm64/signal: Expose GCS state in signal frames Message-ID: References: <20240801-arm64-gcs-v10-0-699e2bd2190b@kernel.org> <20240801-arm64-gcs-v10-24-699e2bd2190b@kernel.org> <7433e3d2-996a-45a0-b917-666a340ad109@sirena.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: D214414003F X-Rspamd-Server: rspam01 X-Stat-Signature: 8pr3gsg5f9hkqzukgi3ghcb6ji5w463k X-HE-Tag: 1723736015-604402 X-HE-Meta: U2FsdGVkX19cRgnKK0nzPviPR5irlLYHopNrBZlz+IircWNG/LmTgY/QcGYFYYVyUv8CP/0S7wi/0toz2226luqWU2lEckrtMlxepbuwN76Cz5cMI8BFtG3i2MyL2FfbufbvLZ9AwpsJrxfrGLTdi/y1mNgB69hYLImqAM6ninat9G0R7fhTZoM56ypcvjD8DxD3fSCpJfwRaneeArHF0S8lacB6+V+xYeR/BAxoOpaO3aDeKeS0DJVNHwAKY9WNUP1ojfN3A3skmCtGSu0NaNGpz8Kz4PIyLE8h4ow/5+qsz0O4Gr2xrmOpppN7jkDK5YljdMEVg8p3qZ4Gb1BX5CCuGfOROoF2O5/DtIdJrOWcmgwogfs6//0qNGBCFZthQFic5jSZp9MaY9h6GnQSAUrr5qtSKqZj/jbukEQEmtr06Y/ik2p/EaZWWzLtaTDqWWQmCfANzuHUdrqJ0zmsW9fjV2X9QsI2JHhNHJjHX8PKdEMsDnNKyIafMePeK+2FGMR3ZdUgOhRzKtY+Qs5UwTAq1gg1Ky6jgYo1Tbk5ZQPQc33Jy22MjG05dnqRGDR9HrjO1SHHlrQv653vJQGVNAeoGw9/kzXCzEcmAG4VjckDGak34tSdZFeipiy4h0c7NhrgEUV5l+Mi8DkXqNWGI3VbzMpc/BupArf+awDmQVd89RINILATYXXP1uFFZXfLDJ0dNQvbPDsKmqlcsXbaMf9bPn61D3U9IsM4DBj4g2KsfVFoN7R8AFkanBcbGB8uB9WKocVOQGccd1DVLTnBJwJt4XKlBk2D2Y6e/OW/bFLqaCCyhD1K6KqKMPLbDGy8BT9F43Oei71TVqLFGmY3kkeCl1FGPXhRNP1oCpWozx3+Psqo7Fk01TCnO9GjRsb/Vtyg8JQA7fcJ1dFS+HR4r4QuTBqxWpoxnU6i2pAYAQZhEbQMC0k/VVGtqLwBfXzLjfID1TXqm1v2XZnAHbF 52fROkh1 k6qbPO5a4saQOE9NHibqjR1gMHzXk+tzxCcI+O/ZhFtIP+DCBudnM0f8rNSjHx/mn3Qyv/2GU4W6jaIhAPRU8kmXiJKXByZ/90n6252/Asn5OIIKQ8bfj+Voej7LlYUXa89aX/yH+NSxh/VnWIkXP+6bsZITw17PyVAmeHuJoEgdjz49Xmw7TgAkgx9B34NVok0IRpdB2jphh2T4sNWmd2gtfF5/YzUXWSwOMwvsksFBOCND6bLiSVptGlzvgWT/ejVmyedWkykT8yebr+SLEE0et1g== 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: On Thu, Aug 15, 2024 at 04:05:32PM +0100, Mark Brown wrote: > On Thu, Aug 15, 2024 at 03:01:21PM +0100, Dave Martin wrote: > > > My thought was that if libc knows about shadow stacks, it is probably > > going to be built to use them too and so would enable shadow stack > > during startup to protect its own code. > > > (Technically those would be independent decisions, but it seems a good > > idea to use a hardening feature if you know about and it is present.) > > > If so, shadow stacks might always get turned on before the main program > > gets a look-in. > > > Or is that not the expectation? > > The expectation (at least for arm64) is that the main program will only > have shadow stacks if everything says it can support them. If the > dynamic linker turns them on during startup prior to parsing the main > executables this means that it should turn them off before actually > starting the executable, taking care to consider any locking of features. Hmm, so we really do get a clear "enable shadow stack" call to the kernel, which we can reasonaly expect won't happen for ancient software? If so, I think dumping the GCS state in the sigframe could be made conditional on that without problems (?) (We could always make it unconditional later if it turn out that that approach breaks something.) > > > > > Is there any scenario where it is legitimate for the signal handler to > > > > change the shadow stack mode or to return with an altered GCSPR_EL0? > > > > If userspace can rewrite the stack pointer on return (eg, to return to a > > > different context as part of userspace threading) then it will need to > > > Do we know if code that actually does that? IIUC, trying to do this is > > totally broken on most arches nowadays; making it work requires a > > reentrant C library and/or logic to defer signals around critical > > sections in userspace. > > > "Real" threading makes this pretty pointless for the most part. > > > Related question: does shadow stack work with ucontext-based coroutines? > > Per-context stacks need to be allocated by the program for that. > > Yes, ucontext based coroutines are the sort of thing I meant when I was > talking about returning to a different context? Ah, right. Doing this asynchronously on the back of a signal (instead of doing a sigreturn) is the bad thing. setcontext() officially doesn't work for this any more, and doing it by hacking or rebuilding the sigframe is extremely hairy and probably a terrible idea for the reasons I gave. > > > be able to also update GCSPR_EL0 to something consistent otherwise > > > attempting to return from the interrupted context isn't going to go > > > well. Changing the mode is a bit more exotic, as it is in general. > > > It's as much to provide information to the signal handler as anything > > > else. Note, the way sigcontext (a.k.a. mcontext).__reserved[] is used by glibc for the ucontext API is inspired by the way the kernel uses it, but not guaranteed to be compatible. For the ucontext API glibc doesn't try to store/restore asynchronous contexts (which is why setcontext() from a signal handler is totally broken), so there is no need to store SVE/SME state and hence lots of free space, so this probably is supportable with shadow stacks -- if there's a way to allocate them. This series would be unaffected either way. (IIRC, the contents of mcontext.__reserved[] is totally incompatible with what the kernel puts in there, and doesn't have the same record structure.) > > > I'm not sure that we should always put information in the signal frame > > that the signal handler can't obtain directly. > > > I guess it's harmless to include this, though. > > If we don't include it then if different ucontexts have different GCS > features enabled we run into trouble on context switch. As outlined above, nowadays you can only use setcontext() on a context obtained from getcontext(). Using setcontext() on a context obtained from a sigframe works by accident or not at all, but in any case coroutines always switch synchronously and don't rely on doing this. (See where setcontext deals with the FPSIMD regs: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/aarch64/setcontext.S;h=ba659438c564dc3bbbb8d6039030e2c492649534;hb=HEAD ) So, overall I think making ucontext coroutines with with GCS is purely a libc matter that is "interesting" here, but we don't need to worry about. > > > > Is the guarded stack considered necessary (or at least beneficial) for > > > > backtracing, or is the regular stack sufficient? > > > > It's potentially beneficial, being less vulnerable to corruption and > > > simpler to parse if all you're interested in is return addresses. > > > Profiling in particular was mentioned, grabbing a linear block of memory > > > will hopefully be less overhead than chasing down the stack. The > > > regular stack should generally be sufficient though. > > > I guess we can't really argue that the shadow stack pointer is > > redundant here though. The whole point of shadow stacks is to make > > things more robust... > > > Just kicking the tyres on the question of whether we need it here, but > > I guess it's hard to make a good case for saying "no". > > Indeed. The general model here is that we don't break userspace that > relies on parses the normal stack (so the GCS is never *necessary*) but > clearly you want to have it. Agreed, but perhaps not in programs that haven't enabled shadow stack? Cheers ---Dave