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 35FFAC25B75 for ; Thu, 23 May 2024 20:51:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C7226B009D; Thu, 23 May 2024 16:51:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 876656B009E; Thu, 23 May 2024 16:51:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 73E636B009F; Thu, 23 May 2024 16:51:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 56A416B009D for ; Thu, 23 May 2024 16:51:08 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id D4152160750 for ; Thu, 23 May 2024 20:51:07 +0000 (UTC) X-FDA: 82150855374.10.B27140D Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) by imf29.hostedemail.com (Postfix) with ESMTP id 05093120002 for ; Thu, 23 May 2024 20:51:05 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=Cxyn3lWg; dmarc=pass (policy=quarantine) header.from=protonmail.com; spf=pass (imf29.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.40.131 as permitted sender) smtp.mailfrom=pobrn@protonmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716497466; 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=18i3/SzNqk7N9vvvs6RCob8+nMoLtrDWnp5u15Ezqno=; b=bidCtgW9nZmzdmFjMPP9jy59+m8suWgNBcmxAzHf2M40JCfK0Zxx/WIgVQ9KwoOVa7cQdw TZCsiUjF+UOFPw+FJzOXDeRp0IkCQCk6+eQFA86HzfaK7elpm7UnVAYKWg/TqAR1nMlaDv /eYhmd6LbaWz/vTIb9PtQBU8M47t4mE= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=Cxyn3lWg; dmarc=pass (policy=quarantine) header.from=protonmail.com; spf=pass (imf29.hostedemail.com: domain of pobrn@protonmail.com designates 185.70.40.131 as permitted sender) smtp.mailfrom=pobrn@protonmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716497466; a=rsa-sha256; cv=none; b=fhRDM6mdAmRbK50y0MCqjlEvL+XXVKbdJ9RFRtUNTib7nR39HOBMEcjyyxZ03MmurkUfHa LwQpShAyuOOPANSoikpZVlzLqSczRoZRaJw3R4CP9H3rfoO0h4urDSRqn2rDj8SnPFx0mA tIOMsAsd8xOWL06pR3Qm1Q4UOY1n2Yo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1716497463; x=1716756663; bh=18i3/SzNqk7N9vvvs6RCob8+nMoLtrDWnp5u15Ezqno=; 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=Cxyn3lWgUdY+wgFWr19GeIF66D7HaGUCKHDj90GGXp2Kz0jYJDIgRWmzVsKtpU+tc T8xFPiTEmpHMGTtf7q8wcjAjnDVuvG+XEl4TKtYEu/Y8M17LHyeaXgaRcLENYE3QOg oN6qTV9IntZhit7ZqNAXvUjxl6lKWw7hrvKufg+UNLgN2kCYZjOAAAN+H9VWxk3SUq pSYeHsUwIziNr1+5ztaLFeZ7u7/jiOquXj82jqKDDodNh6yRnbBodkYqUBQdhTITVG keiIiw0fWWxDSkV7BS0FNcdHKn5Yg33MU7IADNTpQ2vZ/xhKQJ8wWGW/Cl2q/i8yEY CZelDLs5B+SWg== Date: Thu, 23 May 2024 20:50:58 +0000 To: Jeff Xu From: =?utf-8?Q?Barnab=C3=A1s_P=C5=91cze?= Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, dmitry.torokhov@gmail.com, dverkamp@chromium.org, hughd@google.com, jorgelo@chromium.org, skhan@linuxfoundation.org, keescook@chromium.org Subject: Re: [PATCH v1] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` Message-ID: In-Reply-To: References: <20240513191544.94754-1-pobrn@protonmail.com> <20240522162324.0aeba086228eddd8aff4f628@linux-foundation.org> <20240523124521.99a798d645b0939d331d70c1@linux-foundation.org> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 6e2c55039883f8444e85eb2b710969c08b235ce4 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 05093120002 X-Stat-Signature: a8kesg5ewnieb9u5nicunkodh5hdynf6 X-HE-Tag: 1716497465-214471 X-HE-Meta: U2FsdGVkX19d5OURhGMdRpOnAJ3JUz4BItULHRGMf0q+kbgvd993BdziHdtWz9OeXFtcgVrAE6+41vzN4sq/YiLILBZrz0ewNjuOD6qYC+SPsTYXeV7l5A5J+iewO5x7LEQWBkrW7eAOFc1HBOZ70WXYapakzoO9ed509RWbwz2T/wS40wMpskQsq2mlzvPmgHxVc4myKd+hUpyXQZ+Op0w5r57AFLdfWDF+u7/Ic/jV/vEaLbIhS3LY6F7lF7RZwvk6Tu+JcDYfjH26uVPzBAadAMqtG2XV3e56cp04+mOtKnbtYk8eyzmBxVQdsotH9gl6MeGA0CRFAGbYBUSgvX8x+bJvKQXho5sZtk1+Ln9UxyGJnvCvHbix6m/Qdvrm/cxRe6nQGVGxcLM0bOals16afZ/N0evNusmfnB8vHVbpt6qFQQo8CDWYw54dK/2pIJ171T5vgFOWwIElNwYpTCXuO+oIstJg56S53xSPBtpbGna7HmSXfbGC7doZtOt7ocgWHAU5t0syqF1sG4CD9TTGVGyBxQjs8CmUUl8h2pQD9KXQB0lyMinREeVLMzZdHXzXNGW+iE4NdCgvponhRNHMScNMf6LxsH65dv7oE0EW5syTz03ILPGY1VN7DZ1gkvpzA0G2JR1El5J2KNR7dYmlZbn1kN1dsIFTjpFRV/NiFZnLyxUdPINF38N1xVqbvSpFQYnU2tR8Xyw/dV/DWPTPaCyrDZK/ImVIS8sXlAqYNqI6ez+LmW0VZgKFNzLLPbCl4AlltLQ3d4GIn0HUhfk2VN2uuLzIHuaV76YRqhQf3GaVUilT2uXEi18CzFf9cbVGXwZM5wzdc/L4DmJ9+5UrYzXuVQfgOXrtcltaTvLL8DT1T4GWJV3Mo7Zlmw8uWLE4ldm5vLB7hGWukM7Pgx9i4Fl1S+slqp1gHpBhi8sNoRdCls6mtz7OKJChNsj5sPKpRwzcRS3Q6Vp5t5O sX71D201 kdmd5POXs2cdt1W1Xt69CdEezelrZhMcEQWA4fm5FBym6OF04HRBlSSbfYYd/vw2w6qdAOwX6ZGEGpriQPXF+YyE4gq3QQYzUsPrKYJH9lyx3/CYOI6kOOalNHVqz+cOHrNGQ0OcEUqgKNbhlzUdr+6dfdQEati9FfuwSQILPHNGKlHZMD5Jf4gfqJRGsozNMwTpDJ1NqyqYDzQxB6UzluK/EkEIwKxdM6iqJyDmofoDUYRHA5iIF4OjLTinSn1yaWzhIucoOaMaauBEVEoW/J1giWhrBeDfs2bzt3Odrlawe9gynBmdwkC6xblDPzlBVabPnzpXjZbQfyX99JqLTaN4uTnJUWvo/4VRrQv3OF/LIaxgzUiKN3/buD7zWD3+Or+38dG6L3xbF3gQfMiPjkn1kb77FAFi0K8cRpP0l09Llsq/2OVeDVOyyRd7bCu3yYoRZ2Xc0bVDbR0S9Wa//RNUBb2RPWMH08eNlaafE+m2hDHRVGuZzEO4s1Szu6SpEoRYS6AB0gB5BvyLkGKPA01IqrzDPMzkquQn4IeiOkcImV3B1zzigcOT6/ZmGtOTgKhn6+neRCGBmolGo7hq+mctytRgbwjVn2jSBXvv2uS019CfpyLkuDH4oVLhQroyXF5NGIaz4BIye1cOrr4oJj2xtimgBMXRxeGhF 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: 2024. m=C3=A1jus 23., cs=C3=BCt=C3=B6rt=C3=B6k 22:44 keltez=C3=A9ssel, Jeff= Xu =C3=ADrta: > Hi Barnab=C3=A1s >=20 > Is that OK that I work on V2 ? It will be based on your V1 change and > I will also add more test cases. Sure, please go ahead. At the very end of this letter you'll find the commit message that I would have sent in v2, maybe you can salvage some of it. Regards, Barnab=C3=A1s P=C5=91cze >=20 > Thanks > -Jeff >=20 > - >=20 > On Thu, May 23, 2024 at 12:45=E2=80=AFPM Andrew Morton > wrote: > > > > On Wed, 22 May 2024 19:32:35 -0700 Jeff Xu wrote: > > > > > > > > > > It's a change to a userspace API, yes? Please let's have a detaile= d > > > > description of why this is OK. Why it won't affect any existing us= ers. > > > > > > > Unfortunately, this is a breaking change that might break a > > > application if they do below: > > > memfd_create("", MFD_NOEXEC_SEAL) > > > fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE); <-- this will fail in new > > > semantics, due to mfd not being sealable. > > > > > > However, I still think the new semantics is a better, the reason is > > > due to the sysctl: memfd_noexec_scope > > > Currently, when the sysctl is set to MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL > > > kernel adds MFD_NOEXEC_SEAL to memfd_create, and the memfd becomes s= ealable. > > > E.g. > > > When the sysctl is set to MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL > > > The app calls memfd_create("",0) > > > application will get sealable memfd, which might be a surprise to app= lication. > > > > > > If the app doesn't want this behavior, they will need one of two belo= w > > > in current implementation. > > > 1> > > > set the sysctl: memfd_noexec_scope to 0. > > > So the kernel doesn't overwrite the mdmfd_create > > > > > > 2> > > > modify their code to get non-sealable NOEXEC memfd. > > > memfd_create("", MEMFD_NOEXEC_SCOPE_NOEXEC) > > > fcntl(fd, F_ADD_SEALS, F_SEAL_SEAL) > > > > > > The new semantics works better with the sysctl. > > > > > > Since memfd noexec is new, maybe there is no application using the > > > MFD_NOEXEC_SEAL to create > > > sealable memfd. They mostly likely use > > > memfd(MFD_NOEXEC_SEAL|MFD_ALLOW_SEALING) instead. > > > I think it might benefit in the long term with the new semantics. > > > > Yes, it's new so I expect any damage will be small. Please prepare a > > v2 which fully explains/justifies the thinking for this > > non-backward-compatible change and which include the cc:stable. > > > > >=20 --- memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING` `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, currently, it also 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=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]. 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/9eb0b7e5826fc76cad7b025bc46f2= 67d4a8784cb/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.e= u/ Cc: stable@vger.kernel.org Fixes: 105ff5339f498a ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC") Signed-off-by: Barnab=C3=A1s P=C5=91cze Reviewed-by: Jeff Xu Reviewed-by: David Rheinsberg ---