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 24B20C25B75 for ; Fri, 31 May 2024 18:56:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 790C86B00A3; Fri, 31 May 2024 14:56:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 740B16B00A5; Fri, 31 May 2024 14:56:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 608766B00A8; Fri, 31 May 2024 14:56:35 -0400 (EDT) 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 4202F6B00A3 for ; Fri, 31 May 2024 14:56:35 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id D8702120213 for ; Fri, 31 May 2024 18:56:34 +0000 (UTC) X-FDA: 82179597108.02.A44DB6F Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) by imf15.hostedemail.com (Postfix) with ESMTP id 1050DA0006 for ; Fri, 31 May 2024 18:56:31 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=R++cZoet; spf=pass (imf15.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=1717181792; 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=EF2H09lbmyaCypwI54sg4h1wzWqn3R/Ehd3dbT5gr3w=; b=VR+XxcBJHExHTt173vBGHttIoLDK5p5Madv2yj0tGZMlglGC1R8o3wZYiKcFbTnILgaAk/ sxJhk0C4Yavt0BwrhkOYJnL4KEhpp6CvvTlZ0r6IU8uT+Emqsk6YPJkw2g7dARTDG8SRZV l1kh8bS0P7eyHflrSRv182/SV6nopLw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717181792; a=rsa-sha256; cv=none; b=eAldq3Vv3lpZXaM4WGLYybIS8NDvQtkVId0kZMxxm7S/ZpwEKpwZ7Tsdua++voSNFDqAIZ hMz7I7gtroq/dChg5B4hc8ZhiLRp8yyuEGPIDXptyXUYRElBM6N7Ypjd3x1bYwL06DOQiu wqLRf3hyHw9mwWS3NZtAdgzYD6z0Mkc= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=protonmail.com header.s=protonmail3 header.b=R++cZoet; spf=pass (imf15.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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1717181789; x=1717440989; bh=EF2H09lbmyaCypwI54sg4h1wzWqn3R/Ehd3dbT5gr3w=; 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=R++cZoet/HlxcS1UtROgiYK3uqFp81tksJBCGM4M82AQG/p6GOCAZVTkOO3+tm3Q7 FgjmQDMd+qNoKjbXdSZgzhT5U1SkKHBfxP3i8getlQUpSLWaxprF1h68ycnMkPmidO 1AZge7qOctjLg2iFTQF4HzQdDsXpvgcsz3bK7SGwCEOGTIvROYEkDRYbEPxL/CWBgQ G6/Gu1NgBpsXDBwmPVNk9KpaIwocA8ycWPWDu2BJ0YY+PMi/hbhKtVR5EbBPnl1cWa DOdqmlKlf5gd82o4mRNpoIVpwdIMNzSf1TPHh9sCWWYv9d74PngRgoS6VPyFPiBZjJ /rpGGncprRing== Date: Fri, 31 May 2024 18:56:22 +0000 To: Jeff Xu From: =?utf-8?Q?Barnab=C3=A1s_P=C5=91cze?= Cc: David Rheinsberg , Jeff Xu , Andrew Morton , cyphar@cyphar.com, dmitry.torokhov@gmail.com, Daniel Verkamp , hughd@google.com, jorgelo@chromium.org, Kees Cook , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, skhan@linuxfoundation.org, stable@vger.kernel.org Subject: Re: [PATCH v2 1/2] memfd: fix MFD_NOEXEC_SEAL to be non-sealable by default Message-ID: <2fQi6XI7TmRN_qi9xldglgYFujpzMvr0bbkxhYNJhY6VRYjDsyTDqavR6OPU6DNBtCKAPgBVKxV0SMo7WnjUaDit-zxsBZauGavgKzqcNy8=@protonmail.com> In-Reply-To: References: <20240524033933.135049-1-jeffxu@google.com> <20240524033933.135049-2-jeffxu@google.com> <79b3aa3e-bc70-410e-9646-0b6880a4a74b@app.fastmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 10e36116d14daed7d928f575d8afdac320345604 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: 3wq6y7nw7514rpaur3mr7ae9k9tueq7w X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 1050DA0006 X-HE-Tag: 1717181791-242836 X-HE-Meta: U2FsdGVkX19QC/zUIXXTtumZZ3qgmrSvMvjhtUzv9j9p5JRIwsuZrcNboE4h1BveVAfaALE0mYJOkp7gc6rORhXgw/1/WS9XQOcUlwGy93gl06wBKnzmIavQGL/V1JkBwyWBgOYFudzQMYcGJbmMQ15Uq/Zf9tXmMyE8iqSs9EhzMoDzJILz/ATspwnqyOzL6CM1SfxGEnAncCJIBoEowuLizUhMOqKJAYD2gV5Vy2v1avb7HIGA8A4j9py+CZAAAEV5bPiqwkzA1Zn/7plgrussEUg/kfQfSKsPkdQyFErX5JtcjI5Yezm97ovUlzuzX1lNl1K86c0hqaSAPzHLsynwpU/QlgusBATsFbmnxNWRFwlUSMr8gmWKoHgI0KWVO47IGCHzVownP7GHpCZCxNzPyzn0SDZzG11zNSiidsojylCCPGrdBBP3l+ox3wt+EetcGy/u9KwezxMoqPtoLNUT0FvzLznOTTseYaxILuIN0YFjEY6unlr8VGKoxJgnC9xby37tUXZAxDTf3gqmwHoeFgXDCCwxfs8hlqfpP8ILhHKW7vxYeBFGfTgIuFBwkrb2NZAWEjiNc9h8UtBGyoN822FWkq/gtb0HQXk73F4SsEAy/+SqG5mlv6blpnKk0w1CBX8q+nCXRlugBnlWpFOcvnW0iEBthD7woFzTRrzJGX8HBiJOExmnXNL+EQsxJdCwd/soguANznPXi0W0yYlE7pLfAVzkcmNhp7426GQRhHmidztVHfG5r4kq9ElQBRV1i0HmrYQNSNZdTS8sI7OiIDTkWbvB6JigdJiCq9lMvdZGDFHvDczu1aBUGTdQJ/gty7zb8kccFAUAt+3mfcJymaQkXGZj7DIjkfCWSYscdivC6we9zyMe8kHgowcq2LxHvzVFuotJ0fm7fIK5mG9DdYwzV1oUJCQARMx+86KbxJHZmIQ+CktVZjPsjajZaqLYcFH0rv3rWS6oEro U/5OWdaU 317iwaMo7ZFxOgYAzp5OOJUmGisQ9dZXte9J/RhV+5VzSRupU8IgE7IQwFtHY2eTE9qzS2jzm03qjRxIvCZFNDsrG3KwyYQb3FzmRxapgU5zhom2znj14O3C4WeVqpClRfkQ0wremiQmIwSqHeBoGcHmAzFLPNSgLh9CwthDSPgsDEudILiN1EE1GqsdWmpVUB7ZU7+aHvcOCkqwU4EQKaNPSVwqOTutaKh3AHXLUI6Eigz+QYMKWvt7sV5DutmDDyPA9pyZDXwG9zuDlsAYWOekSX6FjXOxqofS4o9dV3utAtk9vA5Ujg2PBrG+liTn+K5oirxz5go0oK0L0KJeJc/2N+9yqFxD/HPW6j86GcGB+zJVze9KQXPqr7M16azVqoPxreePY077mHHpNvhSLR2TyCW1Tk+0fRf1iuoQWiKUj0NQMFZNR0MAXJB/Oxc750Q9ZGTz31WmPZy9n50X+0AyWRxWdzBeVeNWpIp1ImiwuN3oU54bQCVmzgesU7HfdH20eRgHzajeb+Q10IgORw7L7mjMIeBm6At0Vpf5qlVCRLbaSw1DWRdlXCqVF7iQd7CrZ2eQrEa0Yc88Zmmc/SGm6mxlYH6ZaMZp0r4UJj7z5QoYq8bgrjUDv2ITLc93XDBLeJ/uEjyhPDxCyh0wM0PJmbFExREuCLA07wDSPQ5tmwaMY/DejNu3i+9b2C3eQCWw75PgVp0CKiBpq+1J32C3WkPlLQQPyLsX9nS45QHvOFZX9BY7nZkPtvK90zQi7BUfjFQ603R/ytyYz7R+y9TLx4FMhzQff5VBF3gJXLRjwQ4Y= 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 30., cs=C3=BCt=C3=B6rt=C3=B6k 0:24 keltez=C3=A9ssel, Jeff = Xu =C3=ADrta: > On Wed, May 29, 2024 at 2:46=E2=80=AFPM Barnab=C3=A1s P=C5=91cze wrote: > > > > Hi > > > > > > 2024. m=C3=A1jus 29., szerda 23:30 keltez=C3=A9ssel, Jeff Xu =C3=ADrta: > > > > > Hi David and Barnab=C3=A1s > > > > > > On Fri, May 24, 2024 at 7:15=E2=80=AFAM David Rheinsberg wrote: > > > > > > > > Hi > > > > > > > > On Fri, May 24, 2024, at 5:39 AM, jeffxu@chromium.org wrote: > > > > > From: Jeff Xu > > > > > > > > > > By default, memfd_create() creates a non-sealable MFD, unless the > > > > > MFD_ALLOW_SEALING flag is set. > > > > > > > > > > When the MFD_NOEXEC_SEAL flag is initially introduced, the MFD cr= eated > > > > > with that flag is sealable, even though MFD_ALLOW_SEALING is not = set. > > > > > This patch changes MFD_NOEXEC_SEAL to be non-sealable by default, > > > > > unless MFD_ALLOW_SEALING is explicitly set. > > > > > > > > > > This is a non-backward compatible change. However, as MFD_NOEXEC_= SEAL > > > > > is new, we expect not many applications will rely on the nature o= f > > > > > MFD_NOEXEC_SEAL being sealable. In most cases, the application al= ready > > > > > sets MFD_ALLOW_SEALING if they need a sealable MFD. > > > > > > > > This does not really reflect the effort that went into this. Should= n't this be something along the lines of: > > > > > > > > This is a non-backward compatible change. However, MFD_NOEXEC_S= EAL > > > > was only recently introduced and a codesearch revealed no break= ing > > > > users apart from dbus-broker unit-tests (which have a patch pen= ding > > > > and explicitly support this change). > > > > > > > Actually, I think we might need to hold on to this change. With debia= n > > > code search, I found more codes that already use MFD_NOEXEC_SEAL > > > without MFD_ALLOW_SEALING. e.g. systemd [1], [2] [3] > > > > Yes, I have looked at those as well, and as far as I could tell, > > they are not affected. Have I missed something? > > > In the example, the MFD was created then passed into somewhere else > (safe_fork_full, open_serialization_fd, etc.), the scope and usage of > mfd isn't that clear to me, you might have checked all the user cases. > In addition, MFD_NOEXEC_SEAL exists in libc and rust and go lib. I > don't know if debian code search is sufficient to cover enough apps . > There is a certain risk. >=20 > Fundamentally, I'm not convinced that making MFD default-non-sealable > has meaningful benefit, especially when MFD_NOEXEC_SEAL is new. Certainly, there is always a risk, I did not mean to imply that there isn't= . However, I believe this risk is low enough to at least warrant an attempt a= t eliminating this inconsistency. It can always be reverted if it turns out t= hat the effects have been vastly underestimated by me. So I would still like to see this change merged. Regards, Barnab=C3=A1s P=C5=91cze >=20 >=20 > > > > Regards, > > Barnab=C3=A1s > > > > > > > > > > I'm not sure if this will break more applications not-knowingly tha= t > > > have started relying on MFD_NOEXEC_SEAL being sealable. The feature > > > has been out for more than a year. > > > > > > Would you consider my augments in [4] to make MFD to be sealable by d= efault ? > > > > > > At this moment, I'm willing to add a document to clarify that > > > MFD_NOEXEC_SEAL is sealable by default, and that an app that needs > > > non-sealable MFD can set SEAL_SEAL. Because both MFD_NOEXEC_SEAL > > > and vm.memfd_noexec are new, I don't think it breaks the existing > > > ABI, and vm.memfd_noexec=3D0 is there for backward compatibility > > > reasons. Besides, I honestly think there is little reason that MFD > > > needs to be non-sealable by default. There might be few rare cases, > > > but the majority of apps don't need that. On the flip side, the fact > > > that MFD is set up to be sealable by default is a nice bonus for an > > > app - it makes it easier for apps to use the sealing feature. > > > > > > What do you think ? > > > > > > Thanks > > > -Jeff > > > > > > [1] https://codesearch.debian.net/search?q=3DMFD_NOEXEC_SEAL > > > [2] https://codesearch.debian.net/show?file=3Dsystemd_256~rc3-5%2Fsrc= %2Fhome%2Fhomed-home.c&line=3D1274 > > > [3] https://sources.debian.org/src/elogind/255.5-1debian1/src/shared/= serialize.c/?hl=3D558#L558 > > > [4] https://lore.kernel.org/lkml/CALmYWFuPBEM2DE97mQvB2eEgSO9Dvt=3DuO= 9OewMhGfhGCY66Hbw@mail.gmail.com/ > > > > > > > > > > > Additionally, this enhances the useability of pid namespace sysc= tl > > > > > vm.memfd_noexec. When vm.memfd_noexec equals 1 or 2, the kernel w= ill > > > > > add MFD_NOEXEC_SEAL if mfd_create does not specify MFD_EXEC or > > > > > MFD_NOEXEC_SEAL, and the addition of MFD_NOEXEC_SEAL enables the = MFD > > > > > to be sealable. This means, any application that does not desire = this > > > > > behavior will be unable to utilize vm.memfd_noexec =3D 1 or 2 to > > > > > migrate/enforce non-executable MFD. This adjustment ensures that > > > > > applications can anticipate that the sealable characteristic will > > > > > remain unmodified by vm.memfd_noexec. > > > > > > > > > > This patch was initially developed by Barnab=C3=A1s P=C5=91cze, a= nd Barnab=C3=A1s > > > > > used Debian Code Search and GitHub to try to find potential break= ages > > > > > and could only find a single one. Dbus-broker's memfd_create() wr= apper > > > > > is aware of this implicit `MFD_ALLOW_SEALING` behavior, and tries= to > > > > > work around it [1]. This workaround will break. Luckily, this onl= y > > > > > affects the test suite, it does not affect > > > > > the normal operations of dbus-broker. There is a PR with a fix[2]= . In > > > > > addition, David Rheinsberg also raised similar fix in [3] > > > > > > > > > > [1]: > > > > > https://github.com/bus1/dbus-broker/blob/9eb0b7e5826fc76cad7b025b= c46f267d4a8784cb/src/util/misc.c#L114 > > > > > [2]: https://github.com/bus1/dbus-broker/pull/366 > > > > > [3]: > > > > > https://lore.kernel.org/lkml/20230714114753.170814-1-david@readah= ead.eu/ > > > > > > > > > > Cc: stable@vger.kernel.org > > > > > Fixes: 105ff5339f498a ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXE= C") > > > > > Signed-off-by: Barnab=C3=A1s P=C5=91cze > > > > > Signed-off-by: Jeff Xu > > > > > Reviewed-by: David Rheinsberg > > > > > > > > Looks good! Thanks! > > > > David > > >