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 BD701C001DF for ; Wed, 2 Aug 2023 07:58:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 28199280139; Wed, 2 Aug 2023 03:58:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 231D4280112; Wed, 2 Aug 2023 03:58:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0D19F280139; Wed, 2 Aug 2023 03:58:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id EBC4A280112 for ; Wed, 2 Aug 2023 03:58:21 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id BD1AD80A39 for ; Wed, 2 Aug 2023 07:58:21 +0000 (UTC) X-FDA: 81078412002.08.6CB9AB1 Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by imf21.hostedemail.com (Postfix) with ESMTP id D91F91C0007 for ; Wed, 2 Aug 2023 07:58:18 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=readahead.eu header.s=fm3 header.b=bXWTfWLT; dkim=pass header.d=messagingengine.com header.s=fm3 header.b=Hk7lbC2f; dmarc=none; spf=pass (imf21.hostedemail.com: domain of david@readahead.eu designates 64.147.123.25 as permitted sender) smtp.mailfrom=david@readahead.eu ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690963099; 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=hgNvOgWnhU72CHJJjI7j1ZEp7Bx+lB7eT5bN2SgTrYg=; b=yf0Yn/tb0PakK2gTqVCztlN+esbvYGdwHQDrLbMubt4uX+LZSf3Oukh0OtmMhelp72aNS3 Qp68j/y3rUbphgP1Ut1DTPSOFsu1DscFinnr7p5u4ijwrUmr9y3mHbF3rBjEOxAadmvUSI qK/zDzJv9HfUfWnVg2CQ0YJ2eZlfio8= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=readahead.eu header.s=fm3 header.b=bXWTfWLT; dkim=pass header.d=messagingengine.com header.s=fm3 header.b=Hk7lbC2f; dmarc=none; spf=pass (imf21.hostedemail.com: domain of david@readahead.eu designates 64.147.123.25 as permitted sender) smtp.mailfrom=david@readahead.eu ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690963099; a=rsa-sha256; cv=none; b=ll9TbFImRQ/SxEeJTjpaKB4SCh+sk7YYeSU5yjBAsnF0/Vx15GHUhRv0+pTmTXBV+qZ9UY +T2IPkBHlqfsvFupXmtpUUeRn4j+kcSy/7nRY5kS7CZ+tiXDU1XnpSEo8Wj9eKaw+wg9Yy 0w0BgtJJGiKHPNzVmM4qV3RddDtICvE= Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id 3C0CA320069B; Wed, 2 Aug 2023 03:58:16 -0400 (EDT) Received: from imap50 ([10.202.2.100]) by compute6.internal (MEProxy); Wed, 02 Aug 2023 03:58:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=readahead.eu; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1690963095; x=1691049495; bh=hgNvOgWnhU72CHJJjI7j1ZEp7Bx+lB7eT5b N2SgTrYg=; b=bXWTfWLTwbScOLXbA5h+G+00oArw+EjOc3OBeYXRWBQz+AMZJ7F Q9ownNe8/H85vcjuzeZSAsTmoEns2nJgWu2lsl+KHKVju/5XkZtiST181OHgayHt e9OiLwprgMy4K52qw/DukiweP4q/LMDCaHmlAJMdtE0Q5YyksqkDK3Xe1muQL5oQ eCXGHIJgm8gZZ6oFrKU+LcjxupqhmZI49h0ndRsGn8SIZxSz5RHbWe7pNbRrjTzC YoQgmzLyDQGCtZSegezKxiBKX0Rj6rfe7A/+bBW5mYKFHXEwMqRw6Im4aGSGzbh/ 1eVswehFD+5d4zrUSoNqZFHPbF/elmizWkg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1690963095; x=1691049495; bh=hgNvOgWnhU72CHJJjI7j1ZEp7Bx+lB7eT5b N2SgTrYg=; b=Hk7lbC2fBtvFOp3I75OKZL5c17qW1+y/l6GuZ1vxtklo75diNPR R8R53NOH14gRnVPSAkBzLFj67tbO6/oKXEJlPTuJeEcc65HijmZ7FBgXH6WWeokc MkTqOt+WKNQUYuyJ11ZcWzQsTHtxnPNeedJNie+KEpQKrOpXU3H5+6g3VlqjI6k8 HO5ZE2DVQ+rlgtGL3xd7JLomVTckch8kZo1IN4PVnvXiPL9g2iDIS046ncPXe5/j LyWou9knXmvB56f3EU17g2ihrfR0cmJb5xFVJPI1mJ8+PusaOL6+N+djQg9bJtAw WRXyGwNm4YJtRmqQfsKz0So196D5gSW+sgQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrjeejgdduvdegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvfevufgtgfesthhqredtreerjeenucfhrhhomhepfdff rghvihguucfthhgvihhnshgsvghrghdfuceouggrvhhiugesrhgvrggurghhvggrugdrvg huqeenucggtffrrghtthgvrhhnpeejgeeutdeufedtjeffvdfghfdvvdetteejfedtieff keduffeiheeijeehvdekteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmh grihhlfhhrohhmpegurghvihgusehrvggruggrhhgvrggurdgvuh X-ME-Proxy: Feedback-ID: id2994666:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 2C38B1700089; Wed, 2 Aug 2023 03:58:15 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-592-ga9d4a09b4b-fm-defalarms-20230725.001-ga9d4a09b Mime-Version: 1.0 Message-Id: In-Reply-To: References: <20230714114753.170814-1-david@readahead.eu> Date: Wed, 02 Aug 2023 09:56:42 +0200 From: "David Rheinsberg" To: "Jeff Xu" Cc: "Jeff Xu" , linux-kernel@vger.kernel.org, "Andrew Morton" , "Kees Cook" , "Daniel Verkamp" , linux-mm@kvack.org, "Peter Xu" , linux-hardening@vger.kernel.org Subject: Re: [PATCH] memfd: support MFD_NOEXEC alongside MFD_EXEC Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D91F91C0007 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: fq9tmiu68cn3ikau1croiy6x6wkhpdoo X-HE-Tag: 1690963098-762117 X-HE-Meta: U2FsdGVkX1/yryOJ/7JusjZFj/0iXbos/dW80/77s4aL5cTX4he5qFpvimM15liGBVURkr6Etstg9aH+y6fxtl60852A80iucQrnP1UZEsgW7J/4Dm4s1P+weQMV57oU4m7DCMTAeIDWoLdYDFpNYWyxjiMCMP40Hb1w3Z8v0pKnMWgIOv3NeRRIgB6uCYWmAnmQHC3cYH7aVevWbAlq8/R4ywhga9OLdIh2gXb3w7C9yKDk+kvd23fgvPOoQY3yV5iwG53hcH4j9Su4maHqc4w4l930+7baT4yn0FHMJQlckx+ANHnKmOmAplUbM65OXrC6S1UX+YDdU+nXnJyzqUAAsEioAPq95a0lI2n2qQxnqPHR3DRr0xQBcw2X5dO1cTFbxDT+NTHPYKCKz6IcunhBFT0wK87ds++vNd0qqysB3FOohzbUDs+y2muG9zP/ysNECo9PvRxvG32AEOjw6ELV+FmfpDbrSe3tRrZ/mv+phJ/OoyrG1cwCo6MbxtCmo0YrQgt1RvrwPG1JIMIfy6jVnv0Q06DfgaYFY9vK+U1hiQmQbk/DdoFOuWTcyZUY9AMAmWAmOtLUdbGKMb9NAX1VS7tOwHvnwMpsbaV6AQ53He7DE02rVM7mPTw5MVhBWlJ+g/3z/6zdi34k+MveFKUuTe7PlzswP3Oft10ExvBxrwH2JK83fWeWO/KRLjAyqnwotZrKKd/wt3mrA3hqFC8TBi1cgiATkfo1UIT7QS6vXGph5e+3hv0/JNUN58FkuFCk3ZePnZ9p2PxfCPFVOS4IUNgwflBXO54t7Zg84TTaIIbXYX08kJ0kkSlKj7YbQ2j80f8c0Og6Gq6V1YwKjRMu8rpXCotVoySMzIsI7bLO46aJBofgGuPYp2uAEoBQHrtdHZZRRPeLkK4WX1LOAzbTqnQR7jdFZF4q5JpVyBaXasF4JZ3oHwgCS+sQFR1WJm4HrYXA7QJbzb9rGaV XcYLWghn JawNLLgccbPPsSOKoI8q/guwM+o/jxc4HSO6kZ1uBnWgWh/fu/skw2di6AiF87xLyfOu3UmASgdK+ayGyWJQC73T+8nQKkEbaA/c35/ccz4Paj/qg/ilgx+QpqsvGmYXHUZUby3UiOOpa/k6qznkSBLORUWGS4r78NzJB6s3AydVOdLQd8vB3POO0nlz8FEpGLmwzQwEVbVOHUohyVI28IJRef6jbr18C33mmuDYdk4mkC4Nxge0xehAg+iRmsfxFeEIuxPVCsO2tSh4+bPKJKbykJz1qnXL4xAXJm+RiIcKoDDu/+A2KwzwOEVgO5q7ZqS5fnDVVEot/djs= 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: Hi Jeff! On Tue, Aug 1, 2023, at 9:24 PM, Jeff Xu wrote: >> My point is, an application might decide to *not* seal a property, be= cause it knows it has to change it later on. But it might still want to = disable the executable bit initially, so to avoid having executable page= s around that can be exploited. >> > > I understand that. > My argument was this application can do this in two steps, as in my > previous email: > 1> memfd_create(MFD_EXEC) > 2> chmod > > Two system calls back to back isn't too terrible, and I know this > might seem to be not optimized for your user case, I will explain it > later, please read on. Yes, I agree that MFD_NOEXEC would be rather simple to imitate. So this = is mostly a discussion about the intention and side-effects of this work= around, which is also likely why we haven't found an agreement, yet. [...] >> I think I didn't get my point across. Imagine an application that doe= s *NOT* use sealing, but uses memfds. This application shares memfds wit= h untrusted clients, and does this in a safe way (SIGBUS protected). Eve= rything works fine, unless someone decides to enable `vm.memfd_noexec=3D= 2`. Suddenly, the memfd will have sealing enabled *without* the applicat= ion ever requesting this. Now any untrusted client that got the memfd ca= n add seals to the memfd, even though the creator of the memfd did not e= nable sealing. This client can now seal WRITES on the memfd, even though= it really should not be able to do that. >> >> (This is not an hypothetical setup, we have such setups for data shar= ing already) > > Thanks, this helps me understand your point better. > > I'm not convinced that sysctl needs to consider the threat model of > "someone" changing and breaking an application. If we follow that > threat model, there are a lot of other sysctls to worry about. > > Also, in the system that you described, if memfd is handled to an > untrusted process, not only "sealing" can cause damage, but also > chmod, arbitrary rw, imo the right approach is to harden the process > or mechanism of passing the memfd. No. The model I describe is carefully designed to hand out file-descript= ors to inodes that the clients have *no* access to. They cannot run fchm= od(2), unlink(2), etc. All they can do is operate on the open file. And = all access to this shared file is properly guarded against possible dama= ge the other concurrent clients can do. The entire model is already hard= ened against malicious actors. With the new sysctl, a new attack-vector is introduced, which was not po= ssible before. I was *explicitly* told to add `MFD_ALLOW_SEALING` for that exact reason= when introducing memfd_create(2). So I am a bit baffled why it is now o= k to enable sealing behind the users back. I agree that the new sysctl is a root-only option. But I fail to see *wh= y* it implies `MFD_ALLOW_SEALING`? This behavior is not documented nor i= s it explained in the original commit-messages, nor mentioned *anywhere*. >> Thus, setting the security-option `memfd_noexec` *breaks* application= s, because it enables sealing. If `MFD_NOEXEC_SEAL` would *not* imply `M= FD_ALLOW_SEALING`, this would not be an issue. IOW, why does =C2=B4MFD_N= OEXEC_SEAL` clear `F_SEAL_SEAL` even if `MFD_ALLOW_SEALING` is not set? >> > > If MFD_NOEXEC_SEAL is not desired, then it should not be used to > overwrite memfd_create() in this system. > > For the question of why the sysctl adding a seal without application > setting it , the rationale here is, as summary of previous/this > emails: I still think we are not talking about the same thing. I completely unde= rstand why you add the seal! I am just questioning why you *CLEAR* `F_SE= AL_SEAL`? That is, why do you enable `MFD_ALLOW_SEALING` without the use= r requesting it? You could just set `F_SEAL_EXEC` without clearing `F_SE= AL_SEAL`. And then require `MFD_ALLOW_SEALING` on top to clear `F_SEAL_S= EAL`. [...] >> The downside of `MFD_NOEXEC` is that it might be picked over `MFD_NOE= XEC_SEAL` by uneducated users, thus reducing security. But right now, th= e alternative is that existing code picks `MFD_EXEC` instead and never c= lears the executable bit, because it is a hassle to do so. >> > > Yes. This is the downside I was thinking about. > > I lean to believe the kernel API shouldn't be feature rich, it could > be simple, optimized towards the majority of user cases, and ideally, > is self-explained without devs to look through documentation. For > example, if I had to choose one to implement between MFD_NOEXEC and > MFD_NOEXEC_SEAL, I would choose MFD_NOEXEC_SEAL because it should be > what most users care about. Well, if we were to go back, we would make MFD_NOEXEC(_SEAL) the default= and just add `MFD_EXEC` :) >> Or is there another reason *not* to include `MFD_NOEXEC`? I am not su= re I understand fully why you fight it so vehemently? >> > > I wouldn't add it myself, I hope to convince you not to :-). > If you still think it is beneficial to add MFD_NOEXEC (saving one > chmod call and making it easy to use), I wouldn't feel bad about that. > I would suggest going with documentation to help devs to choose > between those two, i.e. recommend MFD_NOEXEC_SEAL in most cases. Any application that cannot use `F_SEAL_EXEC` (e.g., because its peers v= erify for historic reasons that the seal is not set) now has to do an ex= tra dance to get the "safer" behavior, rather than getting the "safer" b= ehavior by default. That is, we make it easier to get the unsafe behavio= r than to get the safe behavior (in this particular scenario). Without `MFD_NOEXEC`, it is easier to end up with a 0777 memfd than not.= I want the application that desires `S_IXUSR` to jump through hoops, no= t the application that does *not* require it. In other words, I would prefer `MFD_ALLOW_EXEC`, which requires fchmod(2= )` to set `S_IXUSR`, rather than requiring a call to fchmod(2) to clear = it for everyone that does not need it. Thanks David