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 AB973CF648A for ; Fri, 27 Sep 2024 22:09:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BEF086B0148; Fri, 27 Sep 2024 18:09:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B9EB56B0149; Fri, 27 Sep 2024 18:09:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A8D496B014A; Fri, 27 Sep 2024 18:09:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 8B22F6B0148 for ; Fri, 27 Sep 2024 18:09:19 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 300D5160EF7 for ; Fri, 27 Sep 2024 22:09:19 +0000 (UTC) X-FDA: 82611910038.15.8E85C9A Received: from mail-4322.protonmail.ch (mail-4322.protonmail.ch [185.70.43.22]) by imf15.hostedemail.com (Postfix) with ESMTP id 5D553A000B for ; Fri, 27 Sep 2024 22:09:17 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=Tx4j2PFR; spf=pass (imf15.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.43.22 as permitted sender) smtp.mailfrom=pobrn@protonmail.com; dmarc=pass (policy=quarantine) header.from=protonmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727474834; 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=IPLRfC8BzQNV6CULWbRBUr4Rw+Tj/CmdqJzBvNMVXNA=; b=Am640XKe5W3Wklq/DNfL1SwJjt9X1vWoQ68HKY6yqmL2PPw3QMLAzWl6wPctz7HCA7yvwg f/2OFLjDvjEnne27L7C9A2G/IOU0gN7jbwkydO8UxfUogO2aCBaEGKkfvQ7vXJuUJ3gi0Z ntvRJUNBRywAPNBFamDBtLML4pZCX88= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727474834; a=rsa-sha256; cv=none; b=VMJVTj27NfZz4RpG2gZYsBaG99YBVXgzkxx9AIXc6/4tnyodyAISP8w4oO4Gegeo9nNg83 agoFmBFP0iAQosMoDutGhZ539QZzt53KlxIBA94hTfl60XFoWc6rTujDuQmbHKz4aAh8yp JIa3LmQxlwc+a0dz9HNV1nVWvbJZp24= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=Tx4j2PFR; spf=pass (imf15.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.43.22 as permitted sender) smtp.mailfrom=pobrn@protonmail.com; dmarc=pass (policy=quarantine) header.from=protonmail.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1727474955; x=1727734155; bh=IPLRfC8BzQNV6CULWbRBUr4Rw+Tj/CmdqJzBvNMVXNA=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=Tx4j2PFRQxMBNSa4HE+J+OvJiC2rBzmY5mfDRY6pWkhrNteWd6fuhROfKcAuJGCbD ryjUQ9pYbQoQnHV8EqVfpKuGBHOuHop/+2OCaalB9lxo9gkuy336EC6P3Rey2KDgeJ ycSmXF93BxAQ7yOyp+h5aAxBIQvaDA4mQFmGXBxPZv/Z+82normoamF02e2fiOTFjY /+2HDTDMUr4krv4JICNingcQ490pYYIMVrmodqyayceyPpFQ49lf8aVX7rYUJpQ9U/ JcqCksau/RyHrm8W1XYooujmYoseEVGs/RWL5d3mZ0l6mmhl7Q1gEtDEglNB0q51XJ QhLtnKVLCbrUw== Date: Fri, 27 Sep 2024 22:09:10 +0000 To: linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, akpm@linux-foundation.org, cyphar@cyphar.com, 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 From: =?utf-8?Q?Barnab=C3=A1s_P=C5=91cze?= Cc: stable@vger.kernel.org Subject: Re: [PATCH v4] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` Message-ID: In-Reply-To: <20240630184912.37335-1-pobrn@protonmail.com> References: <20240630184912.37335-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 5c0df630e371707472ba21da6e6e8ab3c172f12a MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 5D553A000B X-Stat-Signature: 1ynwy93rujwj4x8ice7apgixw3aot8sg X-HE-Tag: 1727474957-109270 X-HE-Meta: U2FsdGVkX19BOiMijSdLeWsKr/nhMrJU1swfnmwf/56yUfAOroaWVpzFui962coyAXGdsrynPE9scJkm+nIu+4jyY420qcPNzKhNmHY/ETjbOtSM2D5AxMmxTAHATtGbIMJtQINNi9NA/d//MI7KbE+vQXPH2IT9rV1vcDmVklNFb2FcckGoBtMOj5JUdCGQoDdEa+Ie/Fl2v9WCuAQ+Z6FsQEwS2fbHvoxUglE5xb8ih98KKRQrX+EKF/6vNmUsirHrqShrPoLuog916OP/GIqpGsqbg62DoCTP/sJaxtIXp5JSui3ZmfieURfhdZHo4JMhTY89ufzndAvzRFkULYPLaEA7o00Z5sProYDU8pZVmky9BFRLF5zLE3ccuDf94naz0AabHvg0sakqbjsu+ZFQ5Px0t9kWkMLqm7Swyn7Pjmx9SU8lVKu/5BGsxN+RppIKNs+O84O1sKQaL8zdOaIe3fUupnEHro1w/wPgrxgPEGN71HRmWy/8irvrFusiUpay2OjkD0lipTl/TEl6BUu+jiHfGAlu0LQ5kWW5XaKFgMmnDuI+88f22/d0wLWuxRvCoTf43iouffj8WHJ47cbnoDu8D2z8fHOyb7qqMB5z+n6uRP0PdrEmM3LBvK9VWMlsCvlTbRq0oEhIbxYC5GaFDnCa8S8wdo7T54xR78OjVagxiL1qfeQrXY2xNCgfyUPyqKdglOZl7Kd0R2RPmBNCuziAql5EP31SAIyN3P1j4qVsfgDzm4Lrle+s3BAjahhFZ9sjs+UpvjvhMgder7tzbdJaT2i/GrVo68O6DZbT3VqyqvsPF6bSUy6nmTCoYFk4xncPJwQIIz97SRcMqB5swUBPcbav9vvYZeNvI96dAteBoEoBLGYIcSaV5LOii29ST3L5lXaRh2VoKkE7jDQ2W+lAIlIZe2e+IQdBsU7llqgqxNmp9fNGqpNssU0LPrmItkGDhaZlttgfxLH iikZFcZH U1Wrr+xBmPRhYaEabaFs6F3zdbi27QWSTytFY6tVGQM+TVpdCLh2CYBngFKYoR2pCUtfqJXu69NcWCuXM9Eb+DSqdG7/Wmm+rTjJvIyn3AmwgGJQxmSuc9VHut7c/QR+VxiB0uNxMaWxiICAlG+H/yqdCVbksstreP94GT5Uv+y6oh30RgOvBdzrt7MylGu9UnfwfKrmH5x3Bp0CQ0fNWKvd/lsHHneLZXWncCsrScR96X/m2ePT1HtUC27mh7/bIW9jxrBnxktLWiWNNJw4gYKDdihkfjO7+wFT8Z2yz6Q4uMWvUPFtBU0uM6W0B0fZ9rf9W7KEZbP0lr1L/r8ZLWu6R1raezoMD2B47bYR0uaGb1yBxdNNXgKmdwiQ7fMK4ZeEOhpolUzSWtu8ZGwirESYzFROaw9A//lr08OnOuqnX7xroCx9P5uAmMhn0DXkbTKOZDhAtiizmP/RhoAeR7BikrTJXtf68kTkFO8tVVfDtXh+PbWLYmfBirb67kNtxEvB26NJRJPrn7MfiPFU3fk1YO6ZInC3lH9wSRmFVmPZRA6K/EgCpukSTMGvH/OlH0wC/GlU43eBRkJ8zLV/cwK8tPOuXsDBdCJg50OpCD1lWRl4paioIGfefKUC1/rwPmw8s0oB5aYjIsVF1dJDX65XPtA== 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: Hi Gentle ping. Is there any chance we could move forward with this? I am not = aware of any breakage it would cause; but longer the wait, the higher the likelih= ood. Regards, Barnab=C3=A1s P=C5=91cze 2024. j=C3=BAnius 30., vas=C3=A1rnap 20:49 keltez=C3=A9ssel, Barnab=C3= =A1s P=C5=91cze =C3=ADrta: > `MFD_NOEXEC_SEAL` should remove the executable bits and set `F_SEAL_EXEC` > to prevent further modifications to the executable bits as per the commen= t > 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. >=20 > 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= .eu/ >=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 > =09=09inode->i_mode &=3D ~0111; > =09=09file_seals =3D memfd_file_seals_ptr(file); > -=09=09if (file_seals) { > -=09=09=09*file_seals &=3D ~F_SEAL_SEAL; > +=09=09if (file_seals) > =09=09=09*file_seals |=3D F_SEAL_EXEC; > -=09=09} > -=09} else if (flags & MFD_ALLOW_SEALING) { > -=09=09/* MFD_EXEC and MFD_ALLOW_SEALING are set */ > +=09} > + > +=09if (flags & MFD_ALLOW_SEALING) { > =09=09file_seals =3D memfd_file_seals_ptr(file); > =09=09if (file_seals) > =09=09=09*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) > =09=09=09 mfd_def_size, > =09=09=09 MFD_CLOEXEC | MFD_NOEXEC_SEAL); > =09mfd_assert_mode(fd, 0666); > -=09mfd_assert_has_seals(fd, F_SEAL_EXEC); > +=09mfd_assert_has_seals(fd, F_SEAL_SEAL | F_SEAL_EXEC); > =09mfd_fail_chmod(fd, 0777); > =09close(fd); > } > --=20 > 2.45.2 >