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 CA728D711CB for ; Wed, 20 Nov 2024 17:20:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5ED176B009A; Wed, 20 Nov 2024 12:20:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 59A9A6B009B; Wed, 20 Nov 2024 12:20:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 462256B009C; Wed, 20 Nov 2024 12:20:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 276D06B009A for ; Wed, 20 Nov 2024 12:20:52 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id BC780160E90 for ; Wed, 20 Nov 2024 17:20:51 +0000 (UTC) X-FDA: 82807135950.08.F000E35 Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) by imf04.hostedemail.com (Postfix) with ESMTP id 75A6A40006 for ; Wed, 20 Nov 2024 17:19:44 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=RLdObkge; spf=pass (imf04.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.40.131 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=1732123004; 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=OlGaza0B5c6KUUDofYWjGR9m+lHpK3J3GBTEAcZ2GV0=; b=buGKYGs1Ft6pU3z+4GQ+kD7/oAzqYjswp5+p4wh/sddQaEYQnrrAbgvEuo+BNM2HHZzzlS 2S0LC+nX+xmdb/xMKbsMbnvpBjhBq2cPzpv0rMv3zdcjD707Uuvq2Q2HYpHRklCgbfJ0CH SkfoaSLes65KzFiWcOotEgIwPghrhZg= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=RLdObkge; spf=pass (imf04.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.40.131 as permitted sender) smtp.mailfrom=pobrn@protonmail.com; dmarc=pass (policy=quarantine) header.from=protonmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732123004; a=rsa-sha256; cv=none; b=nZZ+4RU1G1PwfalItQVn8+oUHqDM4auZSYz9bVTlZyYfFQuBCnsphrh13bapQr/HuTJoCy z9i/heJx19lNSIGw6PSuGLhKxPIR1xZLDDEmfPXIppRHLrFurbngAGIJA4LQ8rdo/WUlz+ MOAzYzxWKms///5CHlojeyhK+ubtr6I= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1732123247; x=1732382447; bh=OlGaza0B5c6KUUDofYWjGR9m+lHpK3J3GBTEAcZ2GV0=; 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:List-Unsubscribe:List-Unsubscribe-Post; b=RLdObkgeYcNfH39o8cuG5d0ikHScQiyIR3eX2RdQ0QVpuwb7UUnp2G+zvSnowFqNK 9K2ONjH5eEAVrw77i8w2qrfb8uku0O1SxaBI9N79ivQmKif+KFy09Mw8gVOIQ4uxwR quSb6YiUY7JrOIJCse3WzAO/Pd1roZLxV97EGXfFtqaYLk/OfJbjqUMnJ0KReppdsp LtX5BRTRmQTuN7UjOEJpJLwgNZt2YOgQh0cZgFlBNtU8Z/t5ZCOdbGQOhLPxLF6sND QL8sA53+RRgHy3od54iAefqrld0j9G0KqUzyJV6Sy+GQFIRRTCW2MpEyk7WOigK/bu iycfY/XlmxnLA== Date: Wed, 20 Nov 2024 17:20:43 +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: References: <20240630184912.37335-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: c499259deb5b64d7a1adf3c1fadcdee5e51a3006 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 75A6A40006 X-Stat-Signature: u9ot5b6tenou467tq3hroesi481fx437 X-Rspam-User: X-HE-Tag: 1732123184-88215 X-HE-Meta: U2FsdGVkX1+NpbHIr2FMyepdxbilioaPwVtbF2Q4F5dYk4u9fI+4Oo3ALya+T0AE5HMH+Iuu0RuXT/m46c2aHqh0LgZl7TV+DebRdaP6QX/qNCO2HNrtfJ8O5q0VEnjLM7U1N0HHulcx0fJS4xExOMrIBML2asfJOkOwDn9CqZYu4yf5c5ILw/uKdaCcH39HF5PUXBkL/YYfFw0xChuwq0w3sYdrMHgRdyhNfcg3kv1shxTM56iwwx+eJU9ZrO3oM8ToKpTtBOyNkg3mbMljagVEElZvPx5XJcI6kf0FCwDu0oCBsqJzUZHk6ubpH6ZLa//A4gjDyXthJm7WlEhMs4pIzGKRKkvQtXlOuTTVD6ASZyf/psocL4Ez5TyLnV8h0sahT5VA1qhdv5CFeVYwS363w+GhRRlWiAUzK+nO7LphWDQyWMqKlOCA6NCcBZtUwWlxVcH5Lr3kfEIWUQ2htrOTuix0zaWrSeM5DRWIOT2qXk3CHOOkZmj07fdvXuWeZrXYQJ9whlouASXd41QrKXNrkfWAJ+BRqO1V54ngtAd5ZT9XJea9YB27dvq0raft2FrmhgEog8KqdOLgF/wswA++Egrh2d2mQix6ySHevsqoOIvAcd8IyuyoOLjc+hlIuOKFoChUVpxJ2QYBrnP4o38cQUXClVM9PRCS7scCyRE8ZCqPdiVcLHsjpnc1cSx5Rf/lcOgVc6qi8chY2zupbeVkVdspJVsm2CTOBFU1/jqxHtXs4dAUs0qtGQMuZvNPh8qHeaSk09DVAxeKIXhAShIHiH2FL+xNFdBiN7g0xCpv2TgtxSZ0xQaObV6UY1FxxWGktIYi5PM+r7eQsTeXbCSpuUbwZHWQMGvkLE991/e/CIcgV+sKJMIu+3CHsHCoCsNdSQEPpmAp1m0HttJPYZOCWVtFMEnepz1hhaoXuOwtyyO9YJQw5L2FXn5nhx9nvWdvs6MuNi7fWFx5cP6 otYJp9oK Qie2Hi2myVNASCO+e9cAhQlIGF3gc2Ea4dQb3yQhDzGsUlZN6lHfCZUS4hEa+6jArWH2/9qiv4jubjfuLci8J2Q9lKOeD/wb1hsewdYRcy/k+Dciv+TLMbzgx0OAsUtqhENu7g3gjpszC1tbEtyWavC/n69qNclVMQKPwLbhqYRHg1nGdTuQSYXbHnadJI/u879HzV2CqnZB447Sk3FVKj7y7/Uk0GXkY8EyD1B8WaAfacdwjJWg90dXoK7nHBD++bawzkUyptiVN8qbvh74fOo14eWzlBputJtL4uYHqzCUSjE56vXUSHlyh8WFthv/0IZ1bh68vlKdEK8cZh0jBHYTtVMMSYHYCDvCDdGDHao8/ZIKQjuFBViRLl4STbKUaZE0aPlmv6rGroaQjNLvNmovz6T7ZNNso/57gGXrz4/T+7w5M99M3BimNJgX3WrZ7l8exMgynSCI/coVtmTTteYcBe227fBH+xLhhm0fz/KtyiiJcY9DSBBjGpx++h6oU5uwyQbX4ycO1cXfJsMVYhy9cKcBg5WVpL/dehyhanPojv6oJuy6PNr5bew3LY3wn2P/SuxXMt92o4IfGtX0vfRNmmpvfb5BERlmwHEmR9J0yctJgaM4CRKpLiNYexf/1mY270/Ho1fCzSgd7iQbwTYZo+A== 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 again. I am still hoping we can move forward with this. Regards, Barnab=C3=A1s P=C5=91cze 2024. szeptember 28., szombat 0:09 keltez=C3=A9ssel, Barnab=C3=A1s P=C5= =91cze =C3=ADrta: > Hi >=20 >=20 > Gentle ping. Is there any chance we could move forward with this? I am no= t aware > of any breakage it would cause; but longer the wait, the higher the likel= ihood. >=20 >=20 > Regards, > Barnab=C3=A1s P=C5=91cze >=20 > 2024. j=C3=BAnius 30., vas=C3=A1rnap 20:49 keltez=C3=A9ssel, Barnab=C3= =A1s P=C5=91cze =C3=ADrta: >=20 > > `MFD_NOEXEC_SEAL` should remove the executable bits and set `F_SEAL_EXE= C` > > to prevent further modifications to the executable bits as per the comm= ent > > in the uapi header file: > > > > not executable and sealed to prevent changing to executable > > > > However, commit 105ff5339f498a ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_= EXEC") > > that introduced this feature made it so that `MFD_NOEXEC_SEAL` unsets > > `F_SEAL_SEAL`, essentially acting as a superset of `MFD_ALLOW_SEALING`. > > > > Nothing implies that it should be so, and indeed up until the second ve= rsion > > of the of the patchset[0] that introduced `MFD_EXEC` and `MFD_NOEXEC_SE= AL`, > > `F_SEAL_SEAL` was not removed, however, it was changed in the third rev= ision > > of the patchset[1] without a clear explanation. > > > > This behaviour is surprising for application developers, there is no > > documentation that would reveal that `MFD_NOEXEC_SEAL` has the addition= al > > effect of `MFD_ALLOW_SEALING`. Additionally, combined with `vm.memfd_no= exec=3D2` > > it has the effect of making all memfds initially sealable. > > > > 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. > > > > 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: > > > > - create memfd: > > - with `MFD_NOEXEC_SEAL`, > > - without `MFD_ALLOW_SEALING`; > > - try to add seals / check the seals. > > > > 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. > > > > 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]. > > > > 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. > > > > There was also a previous attempt to address this peculiarity by > > introducing a new flag[4]. > > > > [0]: https://lore.kernel.org/lkml/20220805222126.142525-3-jeffxu@google= .com/ > > [1]: https://lore.kernel.org/lkml/20221202013404.163143-3-jeffxu@google= .com/ > > [2]: https://github.com/bus1/dbus-broker/blob/9eb0b7e5826fc76cad7b025bc= 46f267d4a8784cb/src/util/misc.c#L114 > > [3]: https://github.com/bus1/dbus-broker/pull/366 > > [4]: https://lore.kernel.org/lkml/20230714114753.170814-1-david@readahe= ad.eu/ > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Barnab=C3=A1s P=C5=91cze > > --- > > > > * v3: https://lore.kernel.org/linux-mm/20240611231409.3899809-1-jeffxu@= chromium.org/ > > * v2: https://lore.kernel.org/linux-mm/20240524033933.135049-1-jeffxu@g= oogle.com/ > > * v1: https://lore.kernel.org/linux-mm/20240513191544.94754-1-pobrn@pro= tonmail.com/ > > > > This fourth version returns to removing the inconsistency as opposed to= documenting > > its existence, with the same code change as v1 but with a somewhat exte= nded commit > > message. This is sent because I believe it is worth at least a try; it = can be easily > > reverted if bigger application breakages are discovered than initially = imagined. > > > > --- > > mm/memfd.c | 9 ++++----- > > tools/testing/selftests/memfd/memfd_test.c | 2 +- > > 2 files changed, 5 insertions(+), 6 deletions(-) > > > > 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, > > > > =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= /selftests/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); > > } > > -- > > 2.45.2 > >