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 6D1D6CED26B for ; Tue, 8 Oct 2024 06:19:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0DEDF6B0089; Tue, 8 Oct 2024 02:19:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 08EB96B008A; Tue, 8 Oct 2024 02:19:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC0A16B008C; Tue, 8 Oct 2024 02:19: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 D103F6B0089 for ; Tue, 8 Oct 2024 02:19:12 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 2D8221A0150 for ; Tue, 8 Oct 2024 06:19:11 +0000 (UTC) X-FDA: 82649432544.20.B7BA5B5 Received: from mail-io1-f42.google.com (mail-io1-f42.google.com [209.85.166.42]) by imf25.hostedemail.com (Postfix) with ESMTP id 8AD8DA0010 for ; Tue, 8 Oct 2024 06:19:10 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=sifive.com header.s=google header.b="RCQV/WPa"; dmarc=pass (policy=reject) header.from=sifive.com; spf=pass (imf25.hostedemail.com: domain of zong.li@sifive.com designates 209.85.166.42 as permitted sender) smtp.mailfrom=zong.li@sifive.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728368202; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=XRr5KbJ5pCun9Ui+yXUQXX9sN3Sf5UKx3OLBX37YeoY=; b=l+V9QecpsDlaf6Wn5vVpPDUGP6Z5JlLQIeRSqFACUN5CjPZ7mQpAVuf+ivB3j3gsW0ATh+ VlLmHox382hAK+pgIsw7KXgf9Rgi053UWoLR5fTPNIGjb3kF7PbOSIc3Tg4c8eQja866wB y1zGuu2nlKaYtRZFJh/8GtQlqeNnxkg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728368202; a=rsa-sha256; cv=none; b=SVXMXBJXMC9OQ1GamVYjlYF0/OYwsr48xfb4f7dkS00J5YIXXS2G0+I7HK/MXT0bUUZQYB x9dUhrTEha3wKIdD3D2hidhUUhDrkf14QBojd8LOHgOpBGi/ImzVjVgZniWBZCBM244MlW GTdpDLnaZdLki8tQl+vKjw1QyNJN9bo= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=sifive.com header.s=google header.b="RCQV/WPa"; dmarc=pass (policy=reject) header.from=sifive.com; spf=pass (imf25.hostedemail.com: domain of zong.li@sifive.com designates 209.85.166.42 as permitted sender) smtp.mailfrom=zong.li@sifive.com Received: by mail-io1-f42.google.com with SMTP id ca18e2360f4ac-83493f2dda4so201515039f.1 for ; Mon, 07 Oct 2024 23:19:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1728368349; x=1728973149; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=XRr5KbJ5pCun9Ui+yXUQXX9sN3Sf5UKx3OLBX37YeoY=; b=RCQV/WPa0okZTU0SoeuJ7docREt5/7VMvc9LoEcXxBfTzNHTJ7Dk55FnOFENWNs2ah iCw+mLf0Tz68JeuUAAoakbPa+mCRzllWMbCUo1CioWhv8Jg+haIR5SS7F0TFt+nucZy+ VBAduObjDdXL/XTB1JOi0UpGlA4uV6GSMNPe7fmE6uvs+jKVSZMFoEQD3yYYZGx8yXAO fFXRr9urYM02cQZKmT+5j+0Koks/Sfm/NwrR9igOa6n2MY2UsRAy4qRUvT6wfBd+xZnF xBHwWhVGOMA12AaynQSPvkPVGHhauKVrNM0eDYN3GsU6Kql2mC8yqqcwDOehg6AwoRSZ oarw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728368349; x=1728973149; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XRr5KbJ5pCun9Ui+yXUQXX9sN3Sf5UKx3OLBX37YeoY=; b=YeZuJ+uaW4GSaEWsLO/TaEm28OVdJxz1HHu+ew5cbFj5QBOW4z5v4ZIpqVs8KeZ5IA W6PjJs37gM24r5VTWX0KYYMv4AgLc8knRih5F1h5Vm55FpzU1wF3YJoiiI/lKF1J2xGW 6+oztYu1Ga69hz2I+TzRqN5uO6rE8CKa2bX3d70pzmG65MoUVJYr1Dt+5lFK55SuLnKa ab9oLuCl7d9s2lPml/nyUG15mt82tqIi26khbVOkKNtZlDssJFW/eAAqJeM3kUar2Mau FZ10cREwTEKX698sVbI7f166yncVB77XmUDE5TcCocbdajEI/v82uOq0U0wXSsDW7V7+ tRSQ== X-Forwarded-Encrypted: i=1; AJvYcCVl2K+GdF9iKT39cw3Ff9rgS2vcMyuRaHW05erUNPYfY19Mu1O5ePy0wUGYvKyp61NhCVKjHljTjg==@kvack.org X-Gm-Message-State: AOJu0YzE8Of+1kaJVfwft2PTGO3CSWpkNYgOUDq+4jCehRXn9CydgDw2 HKhlwbUd8f0gqFpmbhBk1kt+8ilZMl1p8+vTyGfOqMHc0kj7wzQ2KM6aRmv4m3GSqXvC0n/M1qW p8a+XXllzQdvCfbULcetI4SiT5EHO3BzQCW2njQ== X-Google-Smtp-Source: AGHT+IEEV1iKS64xo2W8tSXTVgZ9wK0tVm8MaNOWH+N+f0lrse/SrQLSt9EsGck2HE3GwEbRPJ5ayCLs8lBdAbKu4jQ= X-Received: by 2002:a05:6e02:1a2c:b0:3a0:abd0:122 with SMTP id e9e14a558f8ab-3a38af6a526mr16553285ab.8.1728368349400; Mon, 07 Oct 2024 23:19:09 -0700 (PDT) MIME-Version: 1.0 References: <20241001-v5_user_cfi_series-v1-0-3ba65b6e550f@rivosinc.com> <20241001-v5_user_cfi_series-v1-16-3ba65b6e550f@rivosinc.com> In-Reply-To: From: Zong Li Date: Tue, 8 Oct 2024 14:18:58 +0800 Message-ID: Subject: Re: [PATCH 16/33] riscv/shstk: If needed allocate a new shadow stack on clone 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 , Paul Walmsley , Palmer Dabbelt , Albert Ou , Conor Dooley , Rob Herring , Krzysztof Kozlowski , Arnd Bergmann , Christian Brauner , Peter Zijlstra , Oleg Nesterov , Eric Biederman , Kees Cook , Jonathan Corbet , Shuah Khan , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, alistair.francis@wdc.com, richard.henderson@linaro.org, jim.shu@sifive.com, andybnac@gmail.com, kito.cheng@sifive.com, charlie@rivosinc.com, atishp@rivosinc.com, evan@rivosinc.com, cleger@rivosinc.com, alexghiti@rivosinc.com, samitolvanen@google.com, broonie@kernel.org, rick.p.edgecombe@intel.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 8AD8DA0010 X-Stat-Signature: ofxkrx6bonhgf11wc8or3xrkm7jr716q X-Rspam-User: X-HE-Tag: 1728368350-524807 X-HE-Meta: U2FsdGVkX1/e//ICAHduEh+BSFqQfDmo40Q8qPelFrVF3dfFLHqIEjCloC2BcCQ0QeY10on4A9p8/D1JLS/JXZqlQxmLa3yxgVaMsOfUECKHCJpQudZIebT4GSYOkJ5qdxAXhs/nGi8etfw+9lSwiKPPXapOmq8LTmPeahp1WTa4VurBVDbo8j9veId0r1rZvXZ824jderFwsW8aAWZUcZ1EUzmgbtxGsLjEdeM4eK0JxxUmlNoFyXmOlvtgNqWWzLZomTlN9aQ6weDrzUYnW15yMhmpt18kKHnrvX+zSqJvYRjNOyuGZzKxDlok1t4PjNNGWlwQ1VwVLA2OYlR1pgyPCX9Z2D3YHu+X4a8Iw9VNF1nW00AOBtEOfMigbKNdfuBAwZsLWgVQmXRyzlQyhnXFW4X+SYhq7VLy2f34ot1S0ie6zjMlON8XabSGsPwUFq0qcDznAD+EWR+vas6v862vVqP7lVb7nVNrXweFEHa0Fot78XjOLD74YtLiR+tQQlxwMJ6EHF01FqIgqRgUJ/fzRV8zR0wrSryuUxj1ru7ZC1gKstffhMDDdhRunxK50GlUocgE1p+clppa+Lf3FGdA8nYjfBWPT6EinZcqprkneDASZRFizlG4OeIQ1IOy2mBmZ8ljMfBuf9O1x7HuDK5xH/hFyyEO7JcKYdIiw67kI00JOjdX2cRKTE6msEJaC+bQJxK2sl4vA9QLJzaPvYXk6Pfstt/Ba2CqoPdPRebtRssGEJmqWHOqnRPOYvV25L2TTdy02Ywim3iiGdr63N2BnXO9VPu8CFiu58/lduSlAMqU/e95h577SbhWZacFhimoAbUJ86PiZOxnK5AMpASkEv7+1DerkKwz0YxT7VZI454HHPVvC3GT2EezXoFuIPOhMVL1jrI30Uuu7OF38AOV45WmSVnuViPqeO9y/DjLTyg8BmtwGEkAfg+ja1uqcowY68OOfSCD831NR+g lVxb3u3I wNRyWOyCPlSQ4OCK9xkP5w0RbmpoQRgNgQFkHr+CEKa6gwG9uCqMpAAqmEM41zI9cLzrj8JFOKHIAQRaxIY2NI7gXI6eWZsC213t8OUsfJiQEvGzBvl4ix3qauP4Mpv1go/1VsJ2jNPU+fLZnasrIO3k//nLSpJdauv4Jx51VJOw7rIUusVrXY5S1kXJF4Cj9DGXq4xz1qa2uRU7JS3jDJzCp+zvdqVfnuXsKMqSQJZQRFl8MdJfC/Q6dvLbjQShHyv8gwA3LoViMj2VyhdRdwUtNcycQ5erVc54P50Ri7bYsVXrm9Pl2Z8EDYNJ+jdp8I8jDHb/zcNMeITo= 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 Tue, Oct 8, 2024 at 1:31=E2=80=AFPM Deepak Gupta wr= ote: > > On Tue, Oct 08, 2024 at 01:16:17PM +0800, Zong Li wrote: > >On Tue, Oct 8, 2024 at 7:30=E2=80=AFAM Deepak Gupta = wrote: > >> > >> On Mon, Oct 07, 2024 at 04:17:47PM +0800, Zong Li wrote: > >> >On Wed, Oct 2, 2024 at 12:20=E2=80=AFAM Deepak Gupta wrote: > >> >> > >> >> Userspace specifies CLONE_VM to share address space and spawn new t= hread. > >> >> `clone` allow userspace to specify a new stack for new thread. Howe= ver > >> >> there is no way to specify new shadow stack base address without ch= anging > >> >> API. This patch allocates a new shadow stack whenever CLONE_VM is g= iven. > >> >> > >> >> In case of CLONE_VFORK, parent is suspended until child finishes an= d thus > >> >> can child use parent shadow stack. In case of !CLONE_VM, COW kicks = in > >> >> because entire address space is copied from parent to child. > >> >> > >> >> `clone3` is extensible and can provide mechanisms using which shado= w stack > >> >> as an input parameter can be provided. This is not settled yet and = being > >> >> extensively discussed on mailing list. Once that's settled, this co= mmit > >> >> will adapt to that. > >> >> > >> >> Signed-off-by: Deepak Gupta > >> >> --- > >> >> arch/riscv/include/asm/usercfi.h | 25 ++++++++ > >> > >> ... snipped... > >> > >> >> + > >> >> +/* > >> >> + * This gets called during clone/clone3/fork. And is needed to all= ocate a shadow stack for > >> >> + * cases where CLONE_VM is specified and thus a different stack is= specified by user. We > >> >> + * thus need a separate shadow stack too. How does separate shadow= stack is specified by > >> >> + * user is still being debated. Once that's settled, remove this p= art of the comment. > >> >> + * This function simply returns 0 if shadow stack are not supporte= d or if separate shadow > >> >> + * stack allocation is not needed (like in case of !CLONE_VM) > >> >> + */ > >> >> +unsigned long shstk_alloc_thread_stack(struct task_struct *tsk, > >> >> + const struct kernel_clon= e_args *args) > >> >> +{ > >> >> + unsigned long addr, size; > >> >> + > >> >> + /* If shadow stack is not supported, return 0 */ > >> >> + if (!cpu_supports_shadow_stack()) > >> >> + return 0; > >> >> + > >> >> + /* > >> >> + * If shadow stack is not enabled on the new thread, skip a= ny > >> >> + * switch to a new shadow stack. > >> >> + */ > >> >> + if (is_shstk_enabled(tsk)) > >> > > >> >Hi Deepak, > >> >Should it be '!' is_shstk_enabled(tsk)? > >> > >> Yes it is a bug. It seems like fork without CLONE_VM or with CLONE_VFO= RK, it was returning > >> 0 anyways. And in the case of CLONE_VM (used by pthread), it was not d= oing the right thing. > > > >Hi Deepak, > >I'd like to know if I understand correctly. Could I know whether there > >might also be a risk when the user program doesn't enable the CFI and > >the kernel doesn't activate CFI. Because this flow will still try to > >allocate the shadow stack and execute the ssamowap command. Thanks > > `shstk_alloc_thread_stack` is only called from `copy_thread` and allocat= es and > returns non-zero (positive value) for ssp only if `CLONE_VM` is specified= . > `CLONE_VM` means that address space is shared and userspace has allocated > separate stack. This flow is ensuring that newly created thread with sepa= rate > data stack gets a separate shadow stack as well. > > Retruning zero value from `shstk_alloc_thread_stack` means that, no need = to > allocate a shadow stack. If you look at `copy_thread` function, it simply= sets > the returned ssp in newly created task's task_struct (if it was non-zero)= . > If returned ssp was zero, `copy_thread` doesn't do anything. Thus whateve= r is > current task settings are that will be copied over to new forked/cloned t= ask. > If current task had shadow stack enabled, new task will also get it enabl= ed at > same address (to be COWed later). > > Any task get shadow stack enabled for first time using new prctls (see pr= ctl > patches). > > So only time `ssamoswap` will be exercised will be are > - User issues enabling `prctl` (it'll be issued from loader) > - fork/clone happens > > In both cases, it is guarded against checks of whether cpu supports it an= d task > has shadow stack enabled. > > Let me know if you think I missed any flow. Thanks a lot for the detail, it is very helpful for me. But sorry for the confusion, my question is actually on the situation with this bug (i.e., before the fix) > > > > >> Most of the testing has been with busybox build (independent binaries0= driven via buildroot > >> setup. Wondering why it wasn't caught. > >> > >> Anyways, will fix it. Thanks for catching it. > >> > >> > > >> >> + return 0; > >> >> + > >> >> + /*