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 01F16C369AB for ; Thu, 24 Apr 2025 13:37:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 22EB16B00AD; Thu, 24 Apr 2025 09:36:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1DB766B00AE; Thu, 24 Apr 2025 09:36:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 02F9F6B00AF; Thu, 24 Apr 2025 09:36:58 -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 D49A16B00AD for ; Thu, 24 Apr 2025 09:36:58 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id DBA9512063F for ; Thu, 24 Apr 2025 13:36:59 +0000 (UTC) X-FDA: 83369038158.16.FA5E2F7 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by imf21.hostedemail.com (Postfix) with ESMTP id D36DA1C0010 for ; Thu, 24 Apr 2025 13:36:57 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=ventanamicro.com header.s=google header.b=ToXhWUHy; dmarc=none; spf=pass (imf21.hostedemail.com: domain of rkrcmar@ventanamicro.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=rkrcmar@ventanamicro.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1745501818; 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=u72nfMy9zYS5aW9xBcDq6LESieHcry02Dg/8XEt7OSw=; b=TwUc92c5SWOWreBfzoXdrPAfQa44aIDCOIGGWDTEEOBBciogFFPWareV87lItmopd86xt4 a1MvIqjNLcv8wVYj2vA6fzgFZAD9z9pfJvgS+uWmnTUZnL997NqJ5BNykdpllMiKGkE0HN M1WjCfJVB6Y1bEfd0fJsXMtp4WW/wPg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1745501818; a=rsa-sha256; cv=none; b=Zxecj5v06f5P3cNzLZXiOITeSK++HZft9HIACMgTIUO1aYPvIkTcFh+wQWR2Sv99r2kpOf Xb6Goxq8JGhh8q9Szul0Oz2bQxFFTpO6qzQDuy83uxls8vwuF6zGXFa56W7eufSBufcJt5 FwqRJqEKPctEIYPyPI6WvjwopIg2JN0= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=ventanamicro.com header.s=google header.b=ToXhWUHy; dmarc=none; spf=pass (imf21.hostedemail.com: domain of rkrcmar@ventanamicro.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=rkrcmar@ventanamicro.com Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-39ab85402c9so37078f8f.2 for ; Thu, 24 Apr 2025 06:36:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1745501816; x=1746106616; darn=kvack.org; h=in-reply-to:references:from:to:cc:subject:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=u72nfMy9zYS5aW9xBcDq6LESieHcry02Dg/8XEt7OSw=; b=ToXhWUHyDwWodvWN/iQx8Ha+L5nTfC4C19AOgNab4hgmhoawuXPJGpmjKMwMWUeEga 6fczHVYkOD5B3Po5EDcDGlIQuto3VvqMqgOFdPZOpuENlVT6QfEsDB32d9ozAssK+YmJ unLdj3EYC5g+7foZBs1yNzYIE6fVHSb5uNozlMhArw/0K25D/bNgMxt/Mhvwa4OTouSh 79Q241FN2R9SskUQH219buFH78WXh52jEY15ugWgF04L4jJAUkEB1jJXFSYLcZDZ7iYE 6X00Q5VAntz8u39NqwNI+v/l2a/rWGk7gZxqI+bN5IulKgX3teECoaeukgg1CtnI9rnf wY8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1745501816; x=1746106616; h=in-reply-to:references:from:to:cc:subject:message-id:date :content-transfer-encoding:mime-version:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=u72nfMy9zYS5aW9xBcDq6LESieHcry02Dg/8XEt7OSw=; b=mPHtUfbj2ZRMhgBl3sMqnIm7uqqAiHbaLEHij6ya/qMPyaJcDGFy3DaBCCPhjy8jJL s5ZpRjiqoT5e1om3Gdhw3MfFbL5kB/TqstRGaKlFvcMAvChM9Ca4S6z0jAVDWv7oioTP bFSm6bTma0UdC4c4I6sM5qAROPhZfY2tmKbbIPR83md8fXq4JLgFygaZjKtoquUAefCz /VCzT8aH7f3+lqO8207gGn0mQqTvnC2a757XXJaUnsueL5BC5d61OhRdbTE8OZZ2lk/d G5C6IzuStXeecGcIel7NmzeI0UgqnbcY2zGK+hPAFhO181G11Ayqc18mp/D9Hq6vnl1L qMaQ== X-Forwarded-Encrypted: i=1; AJvYcCUThaakddbP1sex5PY2+sUUDQcPNDazURa71sdymu7J2qIQi+YPsa+zWU60AUPrmBisgsyDoPPk+A==@kvack.org X-Gm-Message-State: AOJu0Yxtd47vPxjlI9DjunBr9aHZmufokD7njXJX1nKx2ACI1AQZj64d j+Srnsxu8xMKg8McgfdVxc6Lh8zrkCY2sdcO4wmNZPiOEWk3KwtcwlzmLa6XPps= X-Gm-Gg: ASbGncsw9m4qdmfjsUYe7ZI/qYqByy5IGm1nIMRtXKUaqnUXzNNlOqOBwEV5CIOTzeu MG/B0yt0UQQ/Tpw69fEiFYvzicmaA7wjTUHMsvz0Wz9x0O66xc9j0l1Kl8EOwB26piQ/2Q8LfMR o7yVgiXCKleqXQI8xRa8/HaZJEkkgLRPYdVk+mRhadiRpXIlJh7GWZUdAfacdtwWwKxct2E8Zbg jcmJwO3JXFGgml4UBKC12aFYBeXD2mKEN9WNwIXTfmGMfUyBnLOqejL5B3/oT9KwUST0O0SlnCs wxbID1Ze4wLJ8MYGejQHzkr9b9jajRFXN0Ir/JQQgbko1h4a X-Google-Smtp-Source: AGHT+IEvTiSK1Lb+RPBdQ/DuDmeOaJEszTrN5CFCiZtbJdEyNkpwKicv/xW9sGpiPTRfU1AmU+RxSg== X-Received: by 2002:a05:6000:2483:b0:3a0:65ab:89d5 with SMTP id ffacd0b85a97d-3a06cfaf02fmr812129f8f.15.1745501815962; Thu, 24 Apr 2025 06:36:55 -0700 (PDT) Received: from localhost ([2a02:8308:a00c:e200:b30c:ee4d:9e10:6a46]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3a06d4c30d7sm2135597f8f.44.2025.04.24.06.36.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Apr 2025 06:36:55 -0700 (PDT) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 24 Apr 2025 15:36:54 +0200 Message-Id: Subject: Re: [PATCH v12 12/28] riscv: Implements arch agnostic shadow stack prctls Cc: "Thomas Gleixner" , "Ingo Molnar" , "Borislav Petkov" , "Dave Hansen" , , "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-riscv" To: "Deepak Gupta" From: =?utf-8?q?Radim_Kr=C4=8Dm=C3=A1=C5=99?= References: <20250314-v5_user_cfi_series-v12-0-e51202b53138@rivosinc.com> <20250314-v5_user_cfi_series-v12-12-e51202b53138@rivosinc.com> In-Reply-To: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: D36DA1C0010 X-Rspam-User: X-Stat-Signature: ywg9xzcedi547n56qmudr6xwx8mm4abu X-HE-Tag: 1745501817-579411 X-HE-Meta: U2FsdGVkX1+35YwdwaVVG1kZhJ38T7qLMMAiJrt+pI6nfcplWL1SCKPmjPCFhKnDCp88YiZFV4mbqefJP7kms723LSjsljAn8mOIrXe7awgdCCsJsVdUlafXbg0+cPJlQX6kBpHP6G+rTnSd+2GhBdNuk1ibsIOk/5pl+sp40uAm+A61QuOQNt9jePiUDaQONz/lbMh20KZvFri/wUtCsrP9IX2PTlt8oQ3esBty6MZcp8kAShfEAWdo5b2nRKbnjRkLuRZ+ORF0Gi77zDSHz/UgrxZeEEdokl7U4Scu1xZ4hpDDxIRjHsSkV2Mms/RTRAjUb6K2HZPD3ZqRCFYkCR7j/gR2G9eLTIigvKknLBak6lm6+OouPjeEgNy317H8xOyY58yVopzqCBDmqmgTXT+SUQi1wJSe1SOTGjQtfisdYwNRlaUytVosOyQ/ax7mEQ87DhJdbWZsRe45qD6ky2vs3C19oNRv5KoRn33yIkFj+0nWad4eRzj7EnujqK568pZyGdIEUBCVj2Eq3n1t3DZRW6I3f8LhEO0Iv0nzmSk2UTLv7ND3OphAs8aEjIEJOp098Rt+zhewIzFThglWpGoBZEPvAR1KG5+FCejutY3cDKCWj91LFtS49FKSAscEX5bWkrgURxegeWrQqvNHSyBb8TKoLprxeQmGQoQdgTbmeHmmNCeD7+Fqo7VPKsMppEzpQ1CVsoACEuXQfLARvEvs6Bg0RqGZh/FCIUp9sg6pc14SPBRu1+mVNoyLz78nzvw8T1ONCa7FdhJRInS0S8nXbSo1FZZQyRe++fpkNEVwdR5xeMxSO/cmGiN/GJrXU1tukjs2+MIzyuVZ4Xpc2DL4/VyW+vembBIH54E8m3Mg2C+WdcA8JEje45OyYIvsvOQhhL4Ka+CXe/6REc2nu6Z6iyrxzNbfZe92dLUOmyv6ivuS4JorOjwv6bG3lQ2StkG7udoKjZcrs/mk8Lg WHGyb4D1 flOCoAJKJDth0N1GP6uWO6HGTIOoOEYAkYnm/iah+KGiGM8oVlU/lr0On6T1bI3aGAn1KhcIutbt+6GA727yJT6e7UvKdWLUnTudaljjYj/I9vojNDdgoWYBCnGBPKScv35mcXS7rpXGULNAdTKNXNfMg2C+oU195MRJnwe1ziTkGbTroLkka4CcJcvVGtoD7INtjAs3M2BMMCXPVl3Z8n8n7jV3KexA0ymzej8ayZYmPZ5eUOsibpZSZDVhgaOGeMp3zW4UydtH+96l5Ll/ugKlXwZBFpoAV68b0MNPUf0CygJeT6qoF8J1ZtmsLRcplteB/c5BsikZEk7UOE9DGgkbf5uknxGma0U9qh/UczJT8kFDtKRcsq0CmaJ9bRd9qvIxL8K3LVxnhwe12OpRNEWlZiA8mds9ZYWraNsE6WSz7gyDp0Mi3cg+wJQDGc9yEGzT6He+ZRmgTAl8t8yTSb59MVIZYirDnc5I+LjTkIjiMYxu/sc57UJJYO/VNBlYo3krWBaPufjooK0cp+WIgJJvKZfpHHtbPIpS2L5owS3tqvSDGD4gOLIHWovTBfxtd9m53YHG3o9df6TXvzKNn4Dlm1X5wqIbbr2kFg535bQkdhhgQkvqFFTdee4k//Ko8TlPsNESvFYpj48JG+jLjaKOrfQ3Q8xE4OCb98V9lOuN244v6GXfOkyUYHg== 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: 2025-04-23T21:44:09-07:00, Deepak Gupta : > On Thu, Apr 10, 2025 at 11:45:58AM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 wro= te: >>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_st= ruct, > don't want to bloat that either. I can just convert it into bitmask if > bitfields are an eyesore here. "unsigned long rsvd : ((sizeof(unsigned long) * 8) - 2);" is an eyesore that defines exactly the same as the two lines alone unsigned long ubcfi_en : 1; unsigned long ubcfi_locked : 1; That one should be removed. If we have only 4 bits in 4/8 bytes, then bitfields do generate worse code than 4 bools and a 0/4 byte hole. The struct size stays the same. I don't care much about the switch to bools, though, because this code is not called often. >>> @@ -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 =3D calc_shstk_size(0); >>> + addr =3D 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 b= y > user task by first issuing `map_shadow_stack`, then asking kernel to ligh= t > up envcfg bit and eventually when return to usermode happens, it can writ= e > 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). arm= 64 > followed the suite. I don't want to find out what's the compatiblity issu= es > 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 siz= e)`. > > 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 i= t > does, it just have to be aware of size its allocating for shadow stack. It's just that userspace cannot prevent allocation of the default stack when enabling it, which is the weird part to me. The allocate and enable syscalls could have been nicely composable. > There is already a patch series going on to manage this using clone3. > https://lore.kernel.org/all/20250408-clone3-shadow-stack-v15-4-3fa245c6e3= be@kernel.org/ A new ioctl does seem to solve most of the practical issues, thanks. > 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. Right, changing rlimit_stack around shadow stack allocation is not the most elegant way, but it does work. >>> +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 he= re */ >>> + if (!cpu_supports_shadow_stack() || >>> + !is_shstk_enabled(task) || arg !=3D 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. Th= ere > are glibc tunables as well using which it can be disabled. The task might do it as some last resort to prevent a buggy code from enabling shadow stacks that would just crash. Or whatever complicated reason userspace can think of. It's more the other way around. I wonder why we're removing this option when we don't really care what userspace does to itself. I think it's complicating the kernel without an obvious gain.