From patchwork Sun Jun 30 18:49:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 13717288 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 919C3C27C4F for ; Sun, 30 Jun 2024 18:49:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C4B856B0088; Sun, 30 Jun 2024 14:49:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BFBC26B008C; Sun, 30 Jun 2024 14:49:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AE9AB6B0092; Sun, 30 Jun 2024 14:49:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 8BBF06B0088 for ; Sun, 30 Jun 2024 14:49:25 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id F16DE1A0DA2 for ; Sun, 30 Jun 2024 18:49:24 +0000 (UTC) X-FDA: 82288443048.04.73BD857 Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) by imf16.hostedemail.com (Postfix) with ESMTP id 3985B180008 for ; Sun, 30 Jun 2024 18:49:22 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=nQULbsA3; spf=pass (imf16.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.43.16 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=1719773342; 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: references:dkim-signature; bh=xnv/xRXqjwS70ESCf+ZLj4/uCgIU1w6IO5pGaPNkan0=; b=7Zj4dfHHySbDpb5cLiw+VDnh/7SLiYO5wBr824+GBanjUifWgUKgqHLnZGjfgqvGI+aOCf Q1X44IO0LI3dan34YHjurH6/z4JzEmFwgK3++jACWQQWQhN4KwOniVwrm+kgzeJw37cf3D OLxO83C3P3lRgZluWtjBc8nzwBMYSWo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719773342; a=rsa-sha256; cv=none; b=R3BJ4D/qVIvy6ynu/6TJ5TG7E+bNCQ7ZUy4b7anFhOQhcC+SK6ZtteTKRhM1J0IIr8J23h dj59x3AOT3pDjFGDQ3XCjjSiM551ZKPBN2jX5NemJeFnAY1gRnz7GvKZLkvx5WoR/V0FYu w8byZEiMFnj1/EYWNdRKuJHP5+Q+5Jo= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=nQULbsA3; spf=pass (imf16.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.43.16 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=1719773360; x=1720032560; bh=xnv/xRXqjwS70ESCf+ZLj4/uCgIU1w6IO5pGaPNkan0=; h=Date:To:From:Cc:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=nQULbsA3WewcSNNPM1MlIEbO+mRWVsHl4kyp+cPsV026zoUyPEgmrwTI5+EzFFqYe HhCwROkKRum8CPwiphhndRsUZk/tAf7NPnTHuBE5/yt2dT08G3qPogp41i7hZtROZy U0kYfGOHcGVb/c7NmZ3xagL0iVu9tB9/8AkOxB6gN/32QSBZOKF5hjBbL2h08M80dv 3T+r3w3p8zYiWlKQ8s4Pass+ItMTEQQJ6rcPOPpy+14aD/tDBvJo6NSF72+2hrOwjY wgqbDKXOs4ZRKJb66c6g7ZrrKVKTBxsC3LgsWpR8YbOa3/1brKUJH02WTCQ+7sL2gE fXqxqLMWqg4Zw== Date: Sun, 30 Jun 2024 18:49:15 +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: [PATCH v4] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` Message-ID: <20240630184912.37335-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 16c190c8f7aecd734f2f341a0a7d6816e61973f2 MIME-Version: 1.0 X-Rspamd-Queue-Id: 3985B180008 X-Stat-Signature: xi4sw7otg7z3zy4nh6g4pfc9bqu31h6g X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1719773362-74825 X-HE-Meta: U2FsdGVkX1/5WYCeElENgIQcLg1Q3cQiUqOFLwXxV2da1gHcnJNjqtxDDbBBo/vKNchTHHZtl6Wfr48JXLbI8Rh2lScIWT0zgSyQUhq8F+qja5RE1NqMPu3HFpXJvYDUAXc0Bc6bugFMDfzZuK9MLdIJITSeWyRSbE++9CPOVhihc2vOD4l626XZp6+n0ZiliSgOeWn79zWzG7wi1xUq1352MVgS0BXzFlonOJlbNpMvDOQVRS0sZsKuVunr8GNFjplTIFP4hT6shyWUrBzTPz2ArtSMZaNzn+7eIrIHWXE9F53g5izPI+mREQpdOllVjhetGmcYD/D4P8dYrCslt9HM8tZD3vBExZ8xo44y0F0rRWcYI63fcyd9+MAzbuu0TC2dzr67wHwt/AdFLNYR1qkB3f/9WQWFJllN28u/4LdCxEBhoWlLL4ZEGEQjDDGC3DODonSXaIWmpxKJUgmrlbqpufnjFR2sfUD9MQBGZc5DU9TbSOFTorGfVvKmgY8LJFAf7lftvsvRTMJB2CeuOOfC8YHEzEV0rl0NXOa3ceCizdwUnozRVDiMjxaRQsgS1lWTgtc2BfeZ/uOiFKTRvVchk54B8JPUpfM/sP5JdLVOyQnTeQTmJcMCoeNNj5YwVTydPzDt49vmqdmdCwqC7SktTQsPI5fqS0wlqUc6WenjE59rXeWJjUSQgOV8ScbJylpNCpH9UCO3VRnbAcdq2l00SuV1BBtDMIRWY7fSlRXpa/aW1UityzFr4Lgf4MQzNUnKc3WK8LpJw4RKDc2YO6icq768B5+sNdaSvXcnUPNpN+xajqM2vncpnKf4MIqGRpQ/FS1EsUqGEzzapKONcd78XZ9gWK/hioia3vC3Eo2JZ2QbYJxhhU21MHGjlziaMPQDJVQTUnD1+SsvMoVg78LVdfYPH32VWkWJSDakZKpOVJ9aKndhYYHVrj1Gfua3FKaw4yDxLBHc8SlsT6y 3Mt4m/WB NTUOLEJR3u5Qk51jDjTwFndaSW0oduUiiPqYBheCVG4TgC+2LrllkNxRTtT5lmgV1Vc2lDNU1x20XJ1Q+KGfVwbJuyOtk9iooUqVH6IPSnXqWyxNRNuH3R9ykFkFk2qvZXJVd28cOyxSpsak/EXMBpediPY/cXqNoipQCstOQTHcb8GIIOCVrlFCIN9ojLrA3WIM1hnLp+EjWXNWZUCtx3fQwN1t1DjudG3t2+08gXXSxLKBAfTpzPCUnz0WDUOdB/i8Y4PY4n6AwbYB2PUwF+CCt//BMVEl46BaMZ2+ZNFClfMVwpHQOY3UYi65lbT33i06Rb0R/FwyRJ1sHPP4DrXe9fzL1w6mgYelBN/uOKjqC5gLecotXgwb3PgD2XdEfpPsPCOMwoAia+DKY5mYY7H4OXv6K57oLJwnfRtwHS3Uu8+xUwoTi1Talkz+91Oa1arrALsxy+qGCbCnkE2i8uGPmz7A4hgRHsvwqjGlBZ7OxFekoDLDwX9NWHgqOa9J9iCOcEUHYLbEm2aEgNHDv1g7XQibGBhBqPtJTRR3Oo0LX9TvV50ZTpVHJK4HxKRahrUnnoWMPmEzkozPtDOydaFBCsmMaqribLM0IaRBIbAuyZS9aRgs3b+jN51P1zoerBgSlGkxIpSoPs1Ul8xvGXI0VZHoKSmsOhZIO4sYT87WEB90/nbTENoD6OQ== 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: `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: 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 version 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 revision 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 additional effect of `MFD_ALLOW_SEALING`. Additionally, combined with `vm.memfd_noexec=2` 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/9eb0b7e5826fc76cad7b025bc46f267d4a8784cb/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/ Cc: stable@vger.kernel.org Signed-off-by: Barnabás Pőcze Reviewed-by: Aleksa Sarai --- * v3: https://lore.kernel.org/linux-mm/20240611231409.3899809-1-jeffxu@chromium.org/ * v2: https://lore.kernel.org/linux-mm/20240524033933.135049-1-jeffxu@google.com/ * v1: https://lore.kernel.org/linux-mm/20240513191544.94754-1-pobrn@protonmail.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 extended 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, inode->i_mode &= ~0111; file_seals = memfd_file_seals_ptr(file); - if (file_seals) { - *file_seals &= ~F_SEAL_SEAL; + if (file_seals) *file_seals |= F_SEAL_EXEC; - } - } else if (flags & MFD_ALLOW_SEALING) { - /* MFD_EXEC and MFD_ALLOW_SEALING are set */ + } + + if (flags & MFD_ALLOW_SEALING) { file_seals = memfd_file_seals_ptr(file); if (file_seals) *file_seals &= ~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) 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); }