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 BBC20C369AB for ; Thu, 24 Apr 2025 04:44:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 798076B0008; Thu, 24 Apr 2025 00:44:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 746356B000A; Thu, 24 Apr 2025 00:44:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5E7146B000C; Thu, 24 Apr 2025 00:44:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 4118D6B0008 for ; Thu, 24 Apr 2025 00:44:17 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 72A30C7840 for ; Thu, 24 Apr 2025 04:44:17 +0000 (UTC) X-FDA: 83367695754.04.6A98D0F Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) by imf14.hostedemail.com (Postfix) with ESMTP id 6BFDE100002 for ; Thu, 24 Apr 2025 04:44:15 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=rivosinc-com.20230601.gappssmtp.com header.s=20230601 header.b=wmPDakp+; dmarc=none; spf=pass (imf14.hostedemail.com: domain of debug@rivosinc.com designates 209.85.210.170 as permitted sender) smtp.mailfrom=debug@rivosinc.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1745469855; a=rsa-sha256; cv=none; b=NINjoaf/ay9KcpT98KBpo+17Zmp+Ocd3sE6ssgCIVTGCl3ni8F1owfyNEVVa8MptS7CNMl 2Hyc0NbKBb6IvsC3qBv38wRgOS2/Kgi8MrBBtIj5cLalZpfq/G2coabGPQKbkIhklK2vaw +BfhbqeskbzaRU3+zhiXh8I4ezRTj54= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=rivosinc-com.20230601.gappssmtp.com header.s=20230601 header.b=wmPDakp+; dmarc=none; spf=pass (imf14.hostedemail.com: domain of debug@rivosinc.com designates 209.85.210.170 as permitted sender) smtp.mailfrom=debug@rivosinc.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1745469855; 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=d8Qiq1brGqi76YM1jZPx/K29TOoSYoqtc1NwyaTJxB8=; b=oO4/jQpEFxWwdupJgoxR31o3/keDt96ndJhv/8e/+KJ1R+T5oIfGNZixeKQOZm3zDHZCAA Oo3Nk20NSO8b95GEwSOjVhprWpGY2Cow/FMJhsAXvUu6n8gEgbA5I5JliVRdqgfgS50cZ5 b4uRWuUehANQDMCKDSncKxbRAiqcnWY= Received: by mail-pf1-f170.google.com with SMTP id d2e1a72fcca58-73c17c770a7so675536b3a.2 for ; Wed, 23 Apr 2025 21:44:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1745469854; x=1746074654; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=d8Qiq1brGqi76YM1jZPx/K29TOoSYoqtc1NwyaTJxB8=; b=wmPDakp+0Q8FXY6ijqxudnpUvzw9jWS9nbh4D5cFZpSLS8W5JjlkkAMfr+VtEupfXe EaExkb3IOneAby9tWyh5+2G5ifBGQov5rZM64LfB/716rDNttDMkt2yi5yNvi1WHcsag wAfyx+uilykZ1f78Mct/MZYZauELBnPMk4Gqxt63DhCe3J/o8cO7WYZtOhudN5d5cdO1 S7UKXBWPCCZO0m/RfW6YqzXVQzxhzkaINWV0vkoqlS/DgjfqBJ29zfEW30E4j2AWPozX Hn1hmkfZfDw9on0GpDTO9YEYMEEUNW+CEGIKcAbrBlvvYDkTT9VoAdqlm/O7iMy5FsMv VFjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1745469854; x=1746074654; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=d8Qiq1brGqi76YM1jZPx/K29TOoSYoqtc1NwyaTJxB8=; b=KmZ3qoJmY+L4yRVY/lJ+IPormEuKGCpP+f6KKZIcobhsAgNuKkkXfrcxGLmB2K//kn F6byz1pd2Tt1bESsAj0mPtq8YH7KxFeLyF8afrP3QMgjDMdWh6rTijMFFMZXD016O2Rt YnSiSNV6K+Z0pK8ApccSeHvz3kdhcMQJLJ0zKOeAvg1BJ9hkrg9MPQiqi9sCxaxSI24N U/O3MG47P8O4SAWa1051mgY2v2zkVDFL3hzWZmU9iceO/4CAZS9i7akHIMlVRNh+l3WK Vc08iXqMYo5oCwzsd4eRewChKudrLLqi/owlTlDp9apKKR8UQ0Vc4sP/6/YuHR6rB+rm pj8g== X-Forwarded-Encrypted: i=1; AJvYcCXy9Z2kbTHU7HNOooS3qOLaZI7KuyIwjW3Uiidh1DpU5+l9dSfNIgITJhGD3wJc7UUx6uNkpD6qwA==@kvack.org X-Gm-Message-State: AOJu0YxwOP4SEAZy2kd3uK3hBpCugCzV68xHY+Q7UDY+QZXNeDfQAx/U VAB2IwjL0/TXEsRQJ42FDq4d8yYoA35Nn4/icBzonf84Y16RXld2kfHC1zDvXCc= X-Gm-Gg: ASbGncvSmP95oJUmUCZjuS6VYOgZtWEcYPCtk9RQNRg2n6fnrsaGnVNFby7JJJJjwNd 7OCfD51xUDpXwOpI1gNEPI2PulojFGdBiHqIW+YLZyvCV75k+/hTsTS97C2SxgIVmvrbnO/qJqc titkdMXMKboz7jRAQnu1dDRqmqW5BiB0vL7jgnkzC2hbtaVRUd9/akbIpOj/xZokj5s1eCgZ+D6 DB3Z5hsQJSJm/wYilUiRh+wHjZw7r4bMNgJIj8yh9GyInvUeJ2Evo90yDDrfskFoHWyc91tDAbF hz6qPXS3XRtgkarTq1gFLs2dpefzd1/uDq/T/ehxWkve9KzF6/E= X-Google-Smtp-Source: AGHT+IEitFVHRTgSCgRhWMe9hU5oQbZR2x7Kpm268qxNcn6QnDdeucUqxqlmgTha+1K/W91b8N7XWQ== X-Received: by 2002:a05:6a00:2182:b0:736:73ad:365b with SMTP id d2e1a72fcca58-73e246647e4mr1767338b3a.14.1745469854052; Wed, 23 Apr 2025 21:44:14 -0700 (PDT) Received: from debug.ba.rivosinc.com ([64.71.180.162]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-73e25a6aa52sm463483b3a.94.2025.04.23.21.44.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Apr 2025 21:44:13 -0700 (PDT) Date: Wed, 23 Apr 2025 21:44:09 -0700 From: Deepak Gupta To: Radim =?utf-8?B?S3LEjW3DocWZ?= 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 , Jann Horn , Conor Dooley , 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, linux-riscv Subject: Re: [PATCH v12 12/28] riscv: Implements arch agnostic shadow stack prctls Message-ID: References: <20250314-v5_user_cfi_series-v12-0-e51202b53138@rivosinc.com> <20250314-v5_user_cfi_series-v12-12-e51202b53138@rivosinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 6BFDE100002 X-Stat-Signature: kws8qefwxduu8n6wh6nx7yue69rjpeqj X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1745469855-337749 X-HE-Meta: U2FsdGVkX19ys+HzgyhSvh3hDotVNBqw6OjmEWuVGjbrboKD3+M9cuw9AmCs5ThaGGhn/V44HN8Rtq/dIzyJVQ5CqsiQxqJSPej4Y54X+UtCAEvhJrtqbPgqpPAFJj6kKaGdXuF0nuYlGIywGdYIhHN25fp3ZIoKKLR7LraEJxziEMBDAkaVXSCnGKrFqlZroKY2KV8bzsFaf0K/kHWVQCqkyONaREX/TIXYmfaJH+BMc06JywAm/xuzLv4JbZcbnYVqDYLv+s1o5wYjeoHxbJR2BdqFVh8oiDQObuaBGGy0F9FHT0QuC5O70v8zry1pkrROL13Vt2iZIIi6u1mh9HLGGhgiHHchaqT598TdZ4pprlkOa+VlrPmnrVOvJoe1x4/iAS80Os38/Jiqy8RzK0ZgyWk6Tjkesh6RGZDJwq0OyYt+7o21zAxxia45m1eGEioQjpR6SZJX/mtZjx+ljqLAODuZVFHbDEcW3dt8/kVaXLYidp8M2fhXK++zsFuyLtEluiNlBZ+ncy9yL/qeMGFtY560DDvvJLq6f/0Qdvba2uSTy6ceQmAFOcOv965lqLq0HQLUNPUl3MepWe5FyPcNpM3iwx+VdSAQblwKfvkMpPHm4VCN4pKjJP5FDcj3wsaRcB295YM3Fk4IQUrfRMy8md8GtVxTLKXs0kaG9UNMkbwrCKUMYFXYhaPfhA87foCUaMRrmlj3EV224+X0JyA9Ul9z7DTT8eiYaNbqfEKB32YjigefXL8M08z5ZABiK2KOYSL/XK/7iIfNWM/Nh9LMiaDH8Al2BAEDo+rSt2epUg2RFGcWidrUY7O+O7RMgeIhIoG0tqTBHZxLQnApmnTUpAh+WV7GgDmLQW97vUvY+VQ+b9QGCKoHV0YOu9SvZDzRQud6xsDeOv7uWBNSvdzrjVc9fSd1MycqXcP8lzR/hukjG4gytY5/7KYJk1vmI1Kq/iFEDBC/FmVunG7 10zysJeh TxJk/DHEr80sHOt4hpq2ss6FGLeReiZQrZeK22a+W/GBS1tKtCzayOIxeSHcIn8rsg0l62XqMn7j1CnJLaQCCwbnI5e5KMDSPK28BQQ3wbnAtUF0pWJQ3cELdiphTizihv14RM5sMhKK2qijc9wSOWS38C1iwyxmL81WDXpL0ACIao5fquM4v9fWlKdHTru23v40HMKu/5FPL3rLWco967xdHtx0XuUmmK5w9aOEX4eMlcDNE83w0YFl0Zj5FowRHbX5BwGya73wfgX7jM8ZlOgEfWtGcqx0K59kqP+xrf5swow8WjbRtjptHK72jl/6lOEKH6ybS7nCSJ+j9o8OEJu3D2TOhzuQ0h9uFuu53gNrYjSCWipjUBwZMJOr5bFNdNZyLlPf1ygM4G8881W9Ibd3qZfxJYb2M9l47L8HLUFaAwy89JhzgsWzdQHtwuBg9Ey35GDsE+joArpzrgtn0eLqoiXk1DDQaCXW6lwZyQbIOPBFURamax/ePXTKTGBvavygmCTnNZSLXReCLmrnUpWg7ml7TotYDhf9XVPhD/hgsboM= 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, Apr 10, 2025 at 11:45:58AM +0200, Radim Krčmář wrote: >2025-03-14T14:39:31-07:00, Deepak Gupta : >> diff --git a/arch/riscv/include/asm/usercfi.h b/arch/riscv/include/asm/usercfi.h >> @@ -14,7 +15,8 @@ struct kernel_clone_args; >> struct cfi_status { >> unsigned long ubcfi_en : 1; /* Enable for backward cfi. */ >> - unsigned long rsvd : ((sizeof(unsigned long) * 8) - 1); >> + unsigned long ubcfi_locked : 1; >> + unsigned long rsvd : ((sizeof(unsigned long) * 8) - 2); > >The rsvd field shouldn't be necessary as the container for the bitfield >is 'unsigned long' sized. > >Why don't we use bools here, though? >It might produce a better binary and we're not hurting for struct size. If you remember one of the previous patch discussion, this goes into `thread_info` Don't want to bloat it. Even if we end shoving into task_struct, don't want to bloat that either. I can just convert it into bitmask if bitfields are an eyesore here. > >> diff --git a/arch/riscv/kernel/usercfi.c b/arch/riscv/kernel/usercfi.c >> @@ -24,6 +24,16 @@ bool is_shstk_enabled(struct task_struct *task) >> +bool is_shstk_allocated(struct task_struct *task) >> +{ >> + return task->thread_info.user_cfi_state.shdw_stk_base ? true : false; > >I think that the following is clearer: > > return task->thread_info.user_cfi_state.shdw_stk_base > >(Similar for all other implicit conversion ternaries.) Hmm... noted. > >> @@ -42,6 +52,26 @@ void set_active_shstk(struct task_struct *task, unsigned long shstk_addr) >> +void set_shstk_status(struct task_struct *task, bool enable) >> +{ >> + if (!cpu_supports_shadow_stack()) >> + return; >> + >> + task->thread_info.user_cfi_state.ubcfi_en = enable ? 1 : 0; >> + >> + if (enable) >> + task->thread.envcfg |= ENVCFG_SSE; >> + else >> + task->thread.envcfg &= ~ENVCFG_SSE; >> + >> + csr_write(CSR_ENVCFG, task->thread.envcfg); > >There is a new helper we could reuse for this: > > envcfg_update_bits(task, ENVCFG_SSE, enable ? ENVCFG_SSE : 0); Yeah it's in switch_to.h header. I'll think about it. > >> +} >> @@ -262,3 +292,83 @@ void shstk_release(struct task_struct *tsk) >> +int arch_set_shadow_stack_status(struct task_struct *t, unsigned long status) >> +{ >> + /* Request is to enable shadow stack and shadow stack is not enabled already */ >> + if (enable_shstk && !is_shstk_enabled(t)) { >> + /* shadow stack was allocated and enable request again >> + * no need to support such usecase and return EINVAL. >> + */ >> + if (is_shstk_allocated(t)) >> + return -EINVAL; >> + >> + size = calc_shstk_size(0); >> + addr = allocate_shadow_stack(0, size, 0, false); > >Why don't we use the userspace-allocated stack? > >I'm completely missing the design idea here... Userspace has absolute >over the shadow stack pointer CSR, so we don't need to do much in Linux: > >1. interface to set up page tables with -W- PTE and >2. interface to control senvcfg.SSE. > >Userspace can do the rest. Design is like following: When a user task wants to enable shadow stack for itself, it has to issue a syscall to kernel (like this prctl). Now it can be done independently by user task by first issuing `map_shadow_stack`, then asking kernel to light up envcfg bit and eventually when return to usermode happens, it can write to CSR. It is no different from doing all of the above together in single `prctl` call. They are equivalent in that nature. Background is that x86 followed this because x86 had workloads/binaries/ functions with (deep)recursive functions and thus by default were forced to always allocate shadow stack to be of the same size as data stack. To reduce burden on userspace for determining and then allocating same size (size of data stack) shadow stack, prctl would do the job of calculating default shadow stack size (and reduce programming error in usermode). arm64 followed the suite. I don't want to find out what's the compatiblity issues we will see and thus just following the suite (given that both approaches are equivalent). Take a look at static `calc_shstk_size(unsigned long size)`. Coming back to your question of why not allowing userspace to manage its own shadow stack. Answer is that it can manage its own shadow stack. If it does, it just have to be aware of size its allocating for shadow stack. There is already a patch series going on to manage this using clone3. https://lore.kernel.org/all/20250408-clone3-shadow-stack-v15-4-3fa245c6e3be@kernel.org/ I fully expect green thread implementations in rust/go or swapcontext based thread management doing this on their own. Current design is to ensure existing apps dont have to change a lot in userspace and by default kernel gives compatibility. Anyone else wanting to optimize the usage of shadow stack can do so with current design. - > >> +int arch_lock_shadow_stack_status(struct task_struct *task, >> + unsigned long arg) >> +{ >> + /* If shtstk not supported or not enabled on task, nothing to lock here */ >> + if (!cpu_supports_shadow_stack() || >> + !is_shstk_enabled(task) || arg != 0) >> + return -EINVAL; > >The task might want to prevent shadow stack from being enabled? But Why would it want to do that? Task can simply not issue the prctl. There are glibc tunables as well using which it can be disabled. > >Thanks.