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 07F87E7716C for ; Thu, 5 Dec 2024 04:29:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2FB3F6B007B; Wed, 4 Dec 2024 23:29:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2ABCC6B0082; Wed, 4 Dec 2024 23:29:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 173456B0083; Wed, 4 Dec 2024 23:29:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id EE3C56B007B for ; Wed, 4 Dec 2024 23:29:08 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9F49C4240F for ; Thu, 5 Dec 2024 04:29:08 +0000 (UTC) X-FDA: 82859624484.17.8E45768 Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) by imf25.hostedemail.com (Postfix) with ESMTP id C5718A0004 for ; Thu, 5 Dec 2024 04:28:55 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=tZPYK4y0; spf=pass (imf25.hostedemail.com: domain of hughd@google.com designates 209.85.210.171 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733372936; 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=LnNG0y2KdMxz8GD2qGxxV98/B5aTxULgFrbHO+ouIYA=; b=chCymawhJSjb1pgY4r9fVrrPbgFfEoXXYFGcGQy95jfMCQpj7KFJwsy+p37h32FFk6lAtB G9uWi5VBr9sxZrCnbvRPsy3kGiewGAFMAp0Z9jM2BYeJf63kb4QftyLrbDwRA8eY9/LJ7Y VOFWaoKE6B9nhsGs2fLXO5OFotVZsfE= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=tZPYK4y0; spf=pass (imf25.hostedemail.com: domain of hughd@google.com designates 209.85.210.171 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733372936; a=rsa-sha256; cv=none; b=6QvAESi7q7go4D9hlmH5sOsnIUr9vGkYy5qxJ93SHxcSVlKOG2BM9LyiNxdEP42YAfZJ86 tjid/R/bpqFsWzdazaK7fYTq05aPtGIC+B4vxBqpslxPIO/M9XoxdByOho5r1jJ0Tk6HyA XBwtTMSqgIElS4Kon79T/2PF2slxvDE= Received: by mail-pf1-f171.google.com with SMTP id d2e1a72fcca58-725958d5ee0so502391b3a.1 for ; Wed, 04 Dec 2024 20:29:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733372945; x=1733977745; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=LnNG0y2KdMxz8GD2qGxxV98/B5aTxULgFrbHO+ouIYA=; b=tZPYK4y0JQDEtLM4yYxYVWXbWUTx5kdsb7mppKufee5bNQSlC8nNQnZK0PAN37yyDQ 6ufXctv69zfCgMC+CA+SYcU9KuayIujN0LF6ek9Ragr0mCc5VjOsZt6dpeHutzS75E4G g0Cj/dw+2rccxRSBV7/sDbUUwQO+pjarW5MMcCAd/u5BgCA4dtywWUga38cPFvqI9tIo XXBiHwEJG1eaW+JULs9E8nOksxPYOgT/bGo7mBqL/iPg7LAA+0M6NLAmK8A7TJ4jML/t znwcs74z8QWkmEdWKKRStex2WQiOxsGuKj6M82p6e9MTQiq3unzh4mkwXauuId5OZ46O ODVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733372945; x=1733977745; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=LnNG0y2KdMxz8GD2qGxxV98/B5aTxULgFrbHO+ouIYA=; b=ZmD8aeY0Lz9n5WcZvX/hTsPiKxRjT+dZue8jIrIZxG94LL1Vbi/mjQZEh4wR+KVbc3 Kv+iraRftC/A+l46OMkh5TKfDSt7rDiAuGZfmPpyjqOJYLQkuB4NSwn15HQEF+awd8zU WzCXIxnEJpXdcuqzr9plMy6BCp2OOXK6qbhgnPpp9tL3CceDtJoxW3SxGiWh7q6oF748 4rELLGcQFaJTV40Ic8oCU5J4WwhxSggiZecnIAJ90fbaK0qKCNtEpeQ6L/SbzNkGhctN MCeqA2r/GWzPqieCH4oNssSYA/b6MNr40/SAdCe7XI5snIO3VniNseSTcNzN4ym4L3xK rZyA== X-Forwarded-Encrypted: i=1; AJvYcCVUjaaY0MvaHGCYCc7/ZabXRGSBrJhGaEFOd2P0g6w5ouIxFl9hbDiXnU6px4ro11aG3QdjwiTxLg==@kvack.org X-Gm-Message-State: AOJu0Yw1ASy7BED35Qk3UBhtjVEt/Vg7jzqKW3mDOBYSxhSKHxcU0nBC 3LDz31VZUZBhw95VVaR8j0pbinZ7eGVAvx0y1oSgk7KKQpely8zp/EGVyEPh9Q== X-Gm-Gg: ASbGncsrBRDBIELbCcv3kCn/ZFE9TfO7Is0LPy2vbT5V8Rj3uW3V5KM2HMWSgW4/GIf lMPlCPYSUi330Me7ZoUrn6oeID0VYwYFLMvKW8VhqBDUE/2AO6Bon5CBd8u/7XQWsHiKuNyjred lgj2qwN4OQ6dxagSM7FtgTvP9pQoMj7YX2NDB1O0bUejAKy9mQfbbRNsuVoNpnUu+tv61tx6z/f PJ0r7G2Rrlhisl9tmwZpt+v3hVoN0ub/ySlj2xl695PLdyJjuOmlw3pf1zGs+HlriUtv0OIFtKI SJOKT5zAa2CD24KSJLkiliwgWQZB3NmFAg== X-Google-Smtp-Source: AGHT+IEzZg9Ycu6ACq6V918QeX+9XdnvPsY4ARwZ5lCe/wSw9M9g/EJq52WOr7BFv+Eht6UWmlBMow== X-Received: by 2002:a05:6a00:1704:b0:71e:744a:3fbc with SMTP id d2e1a72fcca58-7257fcd8eebmr13689101b3a.21.1733372945107; Wed, 04 Dec 2024 20:29:05 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-725a2a90566sm316118b3a.94.2024.12.04.20.29.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Dec 2024 20:29:04 -0800 (PST) Date: Wed, 4 Dec 2024 20:29:02 -0800 (PST) From: Hugh Dickins To: Jann Horn cc: =?UTF-8?Q?Barnab=C3=A1s_P=C5=91cze?= , 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 Subject: Re: [PATCH v4] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` In-Reply-To: Message-ID: <90ecfdb0-6b2a-3f86-b7b7-918d62345ab5@google.com> References: <20240630184912.37335-1-pobrn@protonmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="-1463770367-1189122800-1733372944=:7673" X-Rspamd-Server: rspam05 X-Stat-Signature: jfjxzh8qzo31rbx7aj85nus5rcfsr98k X-Rspamd-Queue-Id: C5718A0004 X-Rspam-User: X-HE-Tag: 1733372935-224659 X-HE-Meta: U2FsdGVkX1/UBb9cwlKy5bXZ9f9WZTRBx+H/sYcSiHt4ucaL987Ay8WEI6TPoURz4h4Wfx0wdFIpzxhpSxPErD7ZOmqhyGFwOgOjJObw46TAemKU9QwAZOnekjLzjIxwOHdg70PgnWq/x9t/eOjlxRduzQqGDB+xJmIRYs4cwnuL0kU7ltOhUh0k1NcRFN4N0Qpgr/07jpH7VkqGfpn9hQRBgSTEVYtHbT5QoJ/mdHLP9SNztIaW1JXe/0lcfGgOeoL8B5QaxzrMtOqkUGqy6tTKQRxWpg+/SmFXB47XJ4q5C9Lajcjtp06HLf0RNwBv1OGnngY26DN+vqTMkvANxFYvL9K09qWSGvVvq5c3n7uTvoWz/GL8zg1xbvFTiiHOpRdFg8LK1IDKpv/uYQQYjfaSPtLV4pP/5jqlEVIybvY/8N/vKDljbvpwjD6AeS3ZXu8it/xjEyoUmVwTIrPqI1kQTvmEIe9z1wwYgH5Wa6qCaqqtfC75/QGZUUkRzapzHs9adUmO4xpb2Q6Jl5tuDzHtPIUGRQl2a2fXTl1Dn0UaVzbPdeWyxlAFL5QFsvjO4rNvT7zYDGlC+zS++ZuyDoCLq8JfqcdPIrj8A31IE/2YAlktJYLNZVMTlO2n/gu1fLDic2u+EZdbVLi8Dubd8obiYlV+P8Dqg1Mf+A2jfCqlL6bkcu5o3wWiHx8R2nfEUXOCCGdpP5rlgp91KZ4HDbASfqfnGfsz0sTnKHL978BLKb8irmZK9mSsc5BWC6lFWfg0784bmTr8nuYVhsj2xftfSuVsLUKdbBDbx6hRmyMw0tXA/27/iwT1t0CNaXVrrhK9lK/3TfiYQ4g5kTi4IJRV1fJYiYWPhQ4cEFhXbEezyTY0rBEg9j88QDwombF9wZmln873Z/Jzlmxt1yOvxD3hN/7MJsnOws+60+aYck+rHG2fQlL82GO76auA9Cpvm/amydFTIbRUcbaRuGm Zl2E9jJP 8Bryq2OXspTcKgETDAzT68hk19y/J3lmvYDU42yjbLACUSwKMKKgbGZBWHWey1gOMvwoeoDwgqfnjeXAoxYnu/37VnKqBCbKcyKINO97XVrTp0BK2nfzSKAvjR4pT2Q1znW93BE/SMyscBnCEMj1dGIUc3HbdAYjJ/7myQqB9PAFxHs5JFiegDI+FomxTS3v4ey3INwfMVFTOKxvMBKrBWxz/o5Q4Zn2eIPp4PDwKJt1Byh7A0dqRcVQu5LORL1FfSO9xRrRTAQe0L+n5/m6VUbuAxOZygpbOocvjlWJ4ifOQx1Vu/GTonprzpQr+L1MtfS1sA3fDFehDIVTo0F8GaMavtWsBw/XkZMIMdufxYofp2icCaghqoLuWN1B8F11Uk/OkbEGRUgEFXazzyi4d2GwB6NIdH+IdLmd5MHaJs+cJQsx4cyqlbphpsJDgTRCrow1bXfrIjwVdYhwxFGEl9LvhUpE81asqKj1o9a1gapJtfETKR/7wx1hGGdsQO30vqG0dzXU7L1mPSb8QpOubHmKTA7bPPIfgIGnqiRGcFxmAG2jRnjqM1TDQzvQprPPZ+bvwRqqasLQT3AAOhyGUOGbKZPSWqkPpPByKk4ep3tzws3VLAZbN4XvWwGmeKYAsD8erhy06JNusy46ixQnA2gbu5z9IlaReb54JtfornAAfjAD8t7Ca4i/+beCDt3/dWLOyFSkWov5Ntdha8Aiiocy+Y+CYpddpICvQ6Hz+/qpVWvg= 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: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. ---1463770367-1189122800-1733372944=:7673 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Jann, I notice you active in the SEALing arena recently, and wonder whether you would would have time to consider Barnab=C3=A1s's patiently pinged and repinged points here. Instinct tells me that he knows what he's talking about: but the SEALing protocol was a little too subtle for me, even at the start, and would take me a bit too long to remaster and comment now (which may well be true of others too). Thanks! Hugh On Wed, 20 Nov 2024, Barnab=C3=A1s P=C5=91cze wrote: > Hi >=20 >=20 > Gentle ping again. I am still hoping we can move forward with this. >=20 >=20 > Regards, > Barnab=C3=A1s P=C5=91cze >=20 >=20 > 2024. szeptember 28., szombat 0:09 keltez=C3=A9ssel, Barnab=C3=A1s P=C5= =91cze =C3=ADrta: >=20 > > Hi > >=20 > >=20 > > 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 lik= elihood. > >=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_E= XEC` > > > to prevent further modifications to the executable bits as per the co= mment > > > in the uapi header file: > > > > > > not executable and sealed to prevent changing to executable > > > > > > However, commit 105ff5339f498a ("mm/memfd: add MFD_NOEXEC_SEAL and MF= D_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 r= evision > > > 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 additi= onal > > > effect of `MFD_ALLOW_SEALING`. Additionally, combined with `vm.memfd_= noexec=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 expecte= d > > > to be minimal. To trigger user visible change, a program has to do th= e > > > 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@goog= le.com/ > > > [1]: https://lore.kernel.org/lkml/20221202013404.163143-3-jeffxu@goog= le.com/ > > > [2]: https://github.com/bus1/dbus-broker/blob/9eb0b7e5826fc76cad7b025= bc46f267d4a8784cb/src/util/misc.c#L114 > > > [3]: https://github.com/bus1/dbus-broker/pull/366 > > > [4]: https://lore.kernel.org/lkml/20230714114753.170814-1-david@reada= head.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-jeffx= u@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@p= rotonmail.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 ex= tended commit > > > message. This is sent because I believe it is worth at least a try; i= t can be easily > > > reverted if bigger application breakages are discovered than initiall= y 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/testi= ng/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 > > > >=20 ---1463770367-1189122800-1733372944=:7673--