linux-kernel - Re: [PATCH v1] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 23 May 2024 10:24:19 +0200
From: "David Rheinsberg" <david@...dahead.eu>
To: Barnabás Pőcze <pobrn@...tonmail.com>,
 "Andrew Morton" <akpm@...ux-foundation.org>
Cc: "Jeff Xu" <jeffxu@...gle.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 dmitry.torokhov@...il.com, "Daniel Verkamp" <dverkamp@...omium.org>,
 hughd@...gle.com, jorgelo@...omium.org, skhan@...uxfoundation.org,
 "Kees Cook" <keescook@...omium.org>
Subject: Re: [PATCH v1] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING`

Hi

On Thu, May 23, 2024, at 4:25 AM, Barnabás Pőcze wrote:
> 2024. május 23., csütörtök 1:23 keltezéssel, Andrew Morton 
> <akpm@...ux-foundation.org> írta:
>> It's a change to a userspace API, yes?  Please let's have a detailed
>> description of why this is OK.  Why it won't affect any existing users.
>
> Yes, it is a uAPI change. To trigger user visible change, a program has to
>
>  - create a memfd
>    - with MFD_NOEXEC_SEAL,
>    - without MFD_ALLOW_SEALING;
>  - try to add seals / check the seals.
>
> This change in essence reverts the kernel's behaviour to that of Linux 
> <6.3, where
> only `MFD_ALLOW_SEALING` enabled sealing. If a program works correctly 
> on those
> kernels, it will likely work correctly after this change.
>
> I have looked through Debian Code Search and GitHub, searching for 
> `MFD_NOEXEC_SEAL`.
> And I could find only a single breakage that this change would case: 
> dbus-broker
> has its own memfd_create() wrapper that is aware of this implicit 
> `MFD_ALLOW_SEALING`
> behaviour[0], and tries to work around it. This workaround will break. 
> Luckily,
> however, as far as I could tell this only affects the test suite of 
> dbus-broker,
> not its normal operations, so I believe it should be fine. I have 
> prepared a PR
> with a fix[1].

We asked for exactly this fix before, so I very much support this. Our test-suite in `dbus-broker` merely verifies what the current kernel behavior is (just like the kernel selftests). I am certainly ok if the kernel breaks it. I will gladly adapt the test-suite.

Previous discussion was in:

    [PATCH] memfd: support MFD_NOEXEC alongside MFD_EXEC
    https://lore.kernel.org/lkml/20230714114753.170814-1-david@readaheadeu/

Note that this fix is particularly important in combination with `vm.memfd_noexec=2`, since this breaks existing user-space by enabling sealing on all memfds unconditionally. I also encourage backporting to stable kernels.

Reviewed-by: David Rheinsberg <david@...dahead.eu>

Thanks
David