Received-SPF: pass (google.com: domain of linux-kernel+bounces-206784-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 07 Jun 2024 21:41:51 +0000
To: jeffxu@chromium.org
From: =?utf-8?Q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>
Cc: 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, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, skhan@linuxfoundation.org, stable@vger.kernel.org
Subject: Re: [PATCH v1 0/1] mm/memfd: add documentation for MFD_NOEXEC_SEAL
Message-ID: <ERhTlU0qgh7_BDdbPy2XWV0pYgJkVYImFQZVPIfvx9F9uyhfaopo8FMZa8WZ9Txx1bzq8qEez4QQ8sOQIwKeQEdn1rym1JgDmvG3zOKdpeQ=@protonmail.com>
In-Reply-To: <20240607203543.2151433-1-jeffxu@google.com>
References: <20240607203543.2151433-1-jeffxu@google.com>
Feedback-ID: 20568564:user:proton
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Hi


2024. j=C3=BAnius 7., p=C3=A9ntek 22:35 keltez=C3=A9ssel, jeffxu@chromium.o=
rg <jeffxu@chromium.org> =C3=ADrta:

> From: Jeff Xu <jeffxu@chromium.org>
>=20
> When MFD_NOEXEC_SEAL was introduced, there was one big mistake: it
> didn't have proper documentation. This led to a lot of confusion,
> especially about whether or not memfd created with the MFD_NOEXEC_SEAL
> flag is sealable. Before MFD_NOEXEC_SEAL, memfd had to explicitly set
> MFD_ALLOW_SEALING to be sealable, so it's a fair question.
>=20
> As one might have noticed, unlike other flags in memfd_create,
> MFD_NOEXEC_SEAL is actually a combination of multiple flags. The idea
> is to make it easier to use memfd in the most common way, which is
> NOEXEC + F_SEAL_EXEC + MFD_ALLOW_SEALING. This works with sysctl
> vm.noexec to help existing applications move to a more secure way of
> using memfd.
>=20
> Proposals have been made to put MFD_NOEXEC_SEAL non-sealable, unless
> MFD_ALLOW_SEALING is set, to be consistent with other flags [1] [2],
> Those are based on the viewpoint that each flag is an atomic unit,
> which is a reasonable assumption. However, MFD_NOEXEC_SEAL was
> designed with the intent of promoting the most secure method of using
> memfd, therefore a combination of multiple functionalities into one
> bit.
>=20
> Furthermore, the MFD_NOEXEC_SEAL has been added for more than one
> year, and multiple applications and distributions have backported and
> utilized it. Altering ABI now presents a degree of risk and may lead
> to disruption.

I feel compelled to mention again that based on my investigation the risk i=
s
minimal. Not to mention that it can easily be reverted if need be.

In my view, it is better to fix the inconsistency than to document it. I wo=
uld
argue that "`MFD_ALLOW_SEALING` is needed to enable sealing except that XYZ=
"
is unintuitive and confusing for a non-significant amount of people.

In conclusion, I think it would be unfortunate if the inconsistency was not=
 fixed and
the problem was considered "solved" by a passing mention in the documentati=
on.


Regards,
Barnab=C3=A1s P=C5=91cze

>=20
> MFD_NOEXEC_SEAL is a new flag, and applications must change their code
> to use it. There is no backward compatibility problem.
>=20
> When sysctl vm.noexec =3D=3D 1 or 2, applications that don't set
> MFD_NOEXEC_SEAL or MFD_EXEC will get MFD_NOEXEC_SEAL memfd. And
> old-application might break, that is by-design, in such a system
> vm.noexec =3D 0 shall be used. Also no backward compatibility problem.
>=20
> I propose to include this documentation patch to assist in clarifying
> the semantics of MFD_NOEXEC_SEAL, thereby preventing any potential
> future confusion.
>=20
> This patch supersede previous patch which is trying different
> direction [3], and please remove [2] from mm-unstable branch when
> applying this patch.
>=20
> Finally, I would like to express my gratitude to David Rheinsberg and
> Barnab=C3=A1s P=C5=91cze for initiating the discussion on the topic of se=
alability.
>=20
> [1]
> https://lore.kernel.org/lkml/20230714114753.170814-1-david@readahead.eu/
>=20
> [2]
> https://lore.kernel.org/lkml/20240513191544.94754-1-pobrn@protonmail.com/
>=20
> [3]
> https://lore.kernel.org/lkml/20240524033933.135049-1-jeffxu@google.com/
>=20
> Jeff Xu (1):
>   mm/memfd: add documentation for MFD_NOEXEC_SEAL MFD_EXEC
>=20
>  Documentation/userspace-api/index.rst      |  1 +
>  Documentation/userspace-api/mfd_noexec.rst | 86 ++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
>  create mode 100644 Documentation/userspace-api/mfd_noexec.rst
>=20
> --
> 2.45.2.505.gda0bf45e8d-goog
>=20
>