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 B627FC3064D for ; Tue, 2 Jul 2024 06:24:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4770E6B0092; Tue, 2 Jul 2024 02:24:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4265A6B0093; Tue, 2 Jul 2024 02:24:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2C7296B0095; Tue, 2 Jul 2024 02:24:48 -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 101ED6B0092 for ; Tue, 2 Jul 2024 02:24:48 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B43151A1B12 for ; Tue, 2 Jul 2024 06:24:47 +0000 (UTC) X-FDA: 82293824214.19.B8EBAA7 Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) by imf23.hostedemail.com (Postfix) with ESMTP id AD8EA14000B for ; Tue, 2 Jul 2024 06:24:45 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=cyphar.com header.s=MBO0001 header.b=Dkuo6Nga; spf=pass (imf23.hostedemail.com: domain of cyphar@cyphar.com designates 80.241.56.152 as permitted sender) smtp.mailfrom=cyphar@cyphar.com; dmarc=pass (policy=reject) header.from=cyphar.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719901469; 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:dkim-signature; bh=Z8uqtxEOVEGEDNB8zOCRNpkE7rC/LoU/VZhdBt7aB2E=; b=Xuhq1G+Ic+BWQCNLuvWXBsn2vc1M51rat6HufjdMUxqacts/8RIHFz+1+9+UFxc30x0jM+ 2091ccpyvw2d8ES6xlRAuM6GONiD7KKykQXU5oMb865mEeF6Qfg3iBkCiAmMdphw6Qunq9 mhbdTD278bqTW6dAhJxmhU4ExQBcN44= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=cyphar.com header.s=MBO0001 header.b=Dkuo6Nga; spf=pass (imf23.hostedemail.com: domain of cyphar@cyphar.com designates 80.241.56.152 as permitted sender) smtp.mailfrom=cyphar@cyphar.com; dmarc=pass (policy=reject) header.from=cyphar.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719901469; a=rsa-sha256; cv=none; b=GQuRMONPha+C5e8pnCeSC4I8jvSlkEar4baWu8wzCceM/WxbGdBL1DAkDERhqg3wAWub8A 8IegqZNOC9rMohjF3bKrgayJ0OvI32ucHOOJdGMCT6CtLMtdMmDxMjOLoJltjCpFMXgkCq mlGh9UlBvXIi3KEoYXIuppr+fi7JFHY= Received: from smtp202.mailbox.org (smtp202.mailbox.org [10.196.197.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4WCtDS4nSkz9smv; Tue, 2 Jul 2024 08:24:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cyphar.com; s=MBO0001; t=1719901480; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Z8uqtxEOVEGEDNB8zOCRNpkE7rC/LoU/VZhdBt7aB2E=; b=Dkuo6NgacKiGuHwkPbRoWTd6TT778a+CuyBlGJHcOF21J10q01ROXqemLTjg3ROdMW1lO2 XlpVhbD6h7Qf5e2LNGj2IcGdpUgBKiC8QvAWDPVj1cNsCtaZRYHVevDzkelzTJaNi3IJgb 3R87CORDjd/C/OWuP9kQSZ4Sp45q5XvsQdIN+vgDhT02LyEQEfRgqS35K9D5r4bd4RWtmv 2D2aof3gWIasLLg9LywnldOTI2aVt4Z1qijqLPiSGIugOgXkyXc8+w79T8spCxoQmijOAu qEMjos7MCBU+Nl1hK62eLDMyABoixcrlG5fTexcY3jT1Ciym9oMUqVrliN8qHg== Date: Mon, 1 Jul 2024 23:24:25 -0700 From: Aleksa Sarai To: =?utf-8?Q?Barnab=C3=A1s_P=C5=91cze?= Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, akpm@linux-foundation.org, david@readahead.eu, dmitry.torokhov@gmail.com, dverkamp@chromium.org, hughd@google.com, jeffxu@google.com, jorgelo@chromium.org, keescook@chromium.org, skhan@linuxfoundation.org, stable@vger.kernel.org Subject: Re: [PATCH v4] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` Message-ID: <20240702.061524-ancient.sardines.voting.pucker-nTYAKsgH2OOw@cyphar.com> References: <20240630184912.37335-1-pobrn@protonmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ahusy5h4jnsdkkjx" Content-Disposition: inline In-Reply-To: <20240630184912.37335-1-pobrn@protonmail.com> X-Stat-Signature: ckxzdjpokoho5ykwcj4f6e64xxdjwprh X-Rspam-User: X-Rspamd-Queue-Id: AD8EA14000B X-Rspamd-Server: rspam02 X-HE-Tag: 1719901485-473765 X-HE-Meta: U2FsdGVkX1+zkEAq4GX1ohXzEBpd8mFfmJXm3FfDLyBs937yPOI4fCTsTX92F4oT5ASHctQwpGzpiv40h0HaLw1mOYzlDzGibHh34bwSn3mgEu0nI7Z5HqB73nPq48+t4ieVN9td4EVb8WLS2uwGWYRtLGacbyJJMVrnUml63HPauJlbOxqeCaJFr4BXpZBywL5qMbz4ch0GLL8vyj3OrAXMkmwlRwLgyzzD8J5vq/TuydW8gaCQtp9Eu/wQWBn/TCH3E8nr+nrlB/Uu2Erye1ZSKubcGVShKAHmzHbCd1IA837ePHgWoWbg8gzdm5vZQiIO1W0UbDJqMg47RYGJYhL07p6D1z6w/JHS81Itu/CnBRHaHepYjioLEnnyliT83pfq7jt164zaUrv91vAre970U+DL1sguZosC5kFxlLOdInG94nm9Y50/xfc78qYR7JA5lzAs3S8dC1747QPL7yxElFs6fDzzFX7C2ja9jrw0QlYP6Z7lm1P4myHd2YeIeUX2FKri69zEEb/n+CJ47QKQ/G1EkUwQUCs0heBoA67anqtdJ7Bqp0WTQbQcnHG2GGUT5Bzp6GGC5vkUArieWicqJ/1rF/wx7bDEGAwEj4x4hdIrdnDaYlwJ1rMl8NQHmzIa7nVaXcbSde6qRmKZS39ri4ny9sgqW3QkknO+azfIYRSQ8C0cYUCXUHHxrEvi0E0+nYSrvn0ctKmC4dt2KQbDUvhaH1YvdoIpxUwGbQycHqbgOKcHmGlliBoIC7JU5M4f/mqAIdkWkPrqUQZ10Mb3EU91iHlJRrpJtLdLEH9Ag92Pr1Ydur1aHJ7IlQaZyiIYspxJRgTyKTTqrdg8o49ehZUyag3Jhvlj5sl/7BaIeKmstswCiyVh/MDRFzQkFoHAwu4+4I0McYt+rtQavtvHEoKUHmfkb2W9Zw4DyLornqb/DiXHvVhNub3gwwaJCT3RftnKIOe80XveByQ P+gDlqya 1nds2s0eU6tYOPUIcpMIyT4qFKqoIcToYAokrvQWINAB/jD9sqw5yL67MIiBxFQptwI2LEZNWrqb/vN7P/dSnilI0W5cVz4vmq846jzIssg3LUgyEtZ1d1FAtuzy+trtKHyBeNMkdcqTdV9qRxiP4dJ1axvMmouccqCslzz8765s9KFL1v7WGe3s3fbagbLkArIhBoM322OKiuwe5g5b6vpZbSRsGaQccddpDHgqsCgIdL02OFK67QP5LiJMHYcbhyCwIHBiLUjsdnhWScU90VFG4LLBd4wEhJTISwV4udP7tYxwOE9lzwQUBOvWpLgLb++oTrJGjwAaA3Ow/XaavCOKFG0iXaxDfymql7X6zsl/+YKLhkm+BM4ct8wqr5QpQoxXUr+L5RpVmi2RD+Bo0LmJnexLp3cjhsdv91EMMeU9Im7Du8B7Erzo+lgZSJPfkTOBGTaORQqqgEj38LjwZ0QVMBQk07upL30SSJMArltzdzRBronQbFwUq5e1Hk37R28WBLpnLZ6ZJO6YZiqUkIxcsLMm+wUpoiqJDszO82d1ytaVqmOoJfGnB49VBCObwTaScZ1iUtiSdadzZhecvYTYc/mBWNJ61POkh6aShTKI1tmWnMWQqqzq6WgzUJo+l5A/xaugWAehbbxHqL7j6EFd6YWzDSHp7W+Q5myQTlCFQe3YqAEerZevYfOgLUJG6tQWEb03z81UWanlLikO+2oGx81Bw6N84ejsfTOxpUtv1W2d6dKM4hCSt/w== 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: --ahusy5h4jnsdkkjx Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2024-06-30, Barnab=C3=A1s P=C5=91cze wrote: > `MFD_NOEXEC_SEAL` should remove the executable bits and set `F_SEAL_EXEC` > to prevent further modifications to the executable bits as per the comment > in the uapi header file: >=20 > not executable and sealed to prevent changing to executable >=20 > However, commit 105ff5339f498a ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EX= EC") > that introduced this feature made it so that `MFD_NOEXEC_SEAL` unsets > `F_SEAL_SEAL`, essentially acting as a superset of `MFD_ALLOW_SEALING`. >=20 > Nothing implies that it should be so, and indeed up until the second vers= ion > of the of the patchset[0] that introduced `MFD_EXEC` and `MFD_NOEXEC_SEAL= `, > `F_SEAL_SEAL` was not removed, however, it was changed in the third revis= ion > of the patchset[1] without a clear explanation. >=20 > This behaviour is surprising for application developers, there is no > documentation that would reveal that `MFD_NOEXEC_SEAL` has the additional > effect of `MFD_ALLOW_SEALING`. Additionally, combined with `vm.memfd_noex= ec=3D2` > it has the effect of making all memfds initially sealable. >=20 > So do not remove `F_SEAL_SEAL` when `MFD_NOEXEC_SEAL` is requested, > thereby returning to the pre-Linux 6.3 behaviour of only allowing > sealing when `MFD_ALLOW_SEALING` is specified. This behaviour makes sense, I'm a little sad I didn't catch it when I was fixing vm.memfd_noexec. There is a possibility for breakage, but we should give it a shot, given how new the API is (and the API itself was also broken until Linux 6.6 anyway)... Feel free to take my Reviewed-by: Aleksa Sarai Thanks. > Now, this is technically a uapi break. However, the damage is expected > to be minimal. To trigger user visible change, a program has to do the > following steps: >=20 > - create memfd: > - with `MFD_NOEXEC_SEAL`, > - without `MFD_ALLOW_SEALING`; > - try to add seals / check the seals. >=20 > But that seems unlikely to happen intentionally since this change > essentially reverts the kernel's behaviour to that of Linux <6.3, > so if a program worked correctly on those older kernels, it will > likely work correctly after this change. >=20 > I have used Debian Code Search and GitHub to try to find potential > breakages, and I could only find a single one. dbus-broker's > memfd_create() wrapper is aware of this implicit `MFD_ALLOW_SEALING` > behaviour, and tries to work around it[2]. This workaround will > break. Luckily, this only affects the test suite, it does not affect > the normal operations of dbus-broker. There is a PR with a fix[3]. >=20 > I also carried out a smoke test by building a kernel with this change > and booting an Arch Linux system into GNOME and Plasma sessions. >=20 > There was also a previous attempt to address this peculiarity by > introducing a new flag[4]. >=20 > [0]: https://lore.kernel.org/lkml/20220805222126.142525-3-jeffxu@google.c= om/ > [1]: https://lore.kernel.org/lkml/20221202013404.163143-3-jeffxu@google.c= om/ > [2]: https://github.com/bus1/dbus-broker/blob/9eb0b7e5826fc76cad7b025bc46= f267d4a8784cb/src/util/misc.c#L114 > [3]: https://github.com/bus1/dbus-broker/pull/366 > [4]: https://lore.kernel.org/lkml/20230714114753.170814-1-david@readahead= =2Eeu/ >=20 > Cc: stable@vger.kernel.org > Signed-off-by: Barnab=C3=A1s P=C5=91cze > --- >=20 > * v3: https://lore.kernel.org/linux-mm/20240611231409.3899809-1-jeffxu@ch= romium.org/ > * v2: https://lore.kernel.org/linux-mm/20240524033933.135049-1-jeffxu@goo= gle.com/ > * v1: https://lore.kernel.org/linux-mm/20240513191544.94754-1-pobrn@proto= nmail.com/ >=20 > This fourth version returns to removing the inconsistency as opposed to d= ocumenting > its existence, with the same code change as v1 but with a somewhat extend= ed commit > message. This is sent because I believe it is worth at least a try; it ca= n be easily > reverted if bigger application breakages are discovered than initially im= agined. >=20 > --- > mm/memfd.c | 9 ++++----- > tools/testing/selftests/memfd/memfd_test.c | 2 +- > 2 files changed, 5 insertions(+), 6 deletions(-) >=20 > diff --git a/mm/memfd.c b/mm/memfd.c > index 7d8d3ab3fa37..8b7f6afee21d 100644 > --- a/mm/memfd.c > +++ b/mm/memfd.c > @@ -356,12 +356,11 @@ SYSCALL_DEFINE2(memfd_create, > =20 > inode->i_mode &=3D ~0111; > file_seals =3D memfd_file_seals_ptr(file); > - if (file_seals) { > - *file_seals &=3D ~F_SEAL_SEAL; > + if (file_seals) > *file_seals |=3D F_SEAL_EXEC; > - } > - } else if (flags & MFD_ALLOW_SEALING) { > - /* MFD_EXEC and MFD_ALLOW_SEALING are set */ > + } > + > + if (flags & MFD_ALLOW_SEALING) { > file_seals =3D memfd_file_seals_ptr(file); > if (file_seals) > *file_seals &=3D ~F_SEAL_SEAL; > diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/s= elftests/memfd/memfd_test.c > index 95af2d78fd31..7b78329f65b6 100644 > --- a/tools/testing/selftests/memfd/memfd_test.c > +++ b/tools/testing/selftests/memfd/memfd_test.c > @@ -1151,7 +1151,7 @@ static void test_noexec_seal(void) > mfd_def_size, > MFD_CLOEXEC | MFD_NOEXEC_SEAL); > mfd_assert_mode(fd, 0666); > - mfd_assert_has_seals(fd, F_SEAL_EXEC); > + mfd_assert_has_seals(fd, F_SEAL_SEAL | F_SEAL_EXEC); > mfd_fail_chmod(fd, 0777); > close(fd); > } > --=20 > 2.45.2 >=20 >=20 --=20 Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH --ahusy5h4jnsdkkjx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQS2TklVsp+j1GPyqQYol/rSt+lEbwUCZoOdGAAKCRAol/rSt+lE b/bMAPsELjqhhIEvgoFUyco4SkEpHUv9AdswlGz0JSGqGog89QD/T9UJTeK56VZR SLv/IwmCkNnuMde9Oc3GeTXAWkIVlAk= =9zpA -----END PGP SIGNATURE----- --ahusy5h4jnsdkkjx--