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] [day] [month] [year] [list]

Message-ID: <CABi2SkUMppyL_LRKJV6BfgGu=1GpGCEOdZ5VHCENMUtzHcRTkA@mail.gmail.com>
Date: Fri, 7 Jun 2024 08:58:08 -0700
From: Jeff Xu <jeffxu@...omium.org>
To: David Rheinsberg <david@...dahead.eu>
Cc: Jeff Xu <jeffxu@...gle.com>, Aleksa Sarai <cyphar@...har.com>, 
	Barnabás Pőcze <pobrn@...tonmail.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, 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 David,

On Fri, Jun 7, 2024 at 1:38 AM David Rheinsberg <david@...dahead.eu> wrote:
>
> Hi
>
> On Tue, May 28, 2024, at 7:13 PM, Jeff Xu wrote:
> >> > Another solution is to change memfd to be by-default sealable,
> >> > although that will be an api change, but what side effect  will it be
> >> > ?
> >> > If we are worried about the memfd being sealed by an attacker, the
> >> > malicious code could also overwrite the content since memfd is not
> >> > sealed.
> >>
> >> You cannot change the default-seals retrospectively. There are existing shmem-users that share file-descriptors and *expect* the other party to be able to override data, but do *not* expect the other party to be able to apply seals. Note that these models explicitly *want* shared, writable access to the buffer (e.g., render-client shares a buffer with the display server for scanout), so just because you can *write* to a shmem-file does not mean that sharing is unsafe (e.g., using SIGBUS+mmap can safely deal with page-faults).
> >>
> > If the other party is controlled by an attacker, the attacker can
> > write garbage to the shm-file/memfd, that is already the end of the
> > game, at that point, sealing is no longer a concern, right?
>
> No. If a graphics client shares a buffer with a graphics server, the client is free to write garbage into the buffer. This is not unsafe. The graphics server will display whatever the client writes into the buffer. This is completely safe, without sealing and with a writable buffer.
>
> > If the threat-model is preventing attacker on the other side to write
> > the garbage data, then F_SEAL_WRITE|F_SEAL_SHRINK|F_SEAL_GROW can be
> > applied, in that case, default-sealable seems preferable because of
> > less code change.
>
> Again, the threat-model is *NOT* concerned with writes.
>
> Graphics clients/servers are a good example where *ANY* data is valid and can be processed by the privileged server. Hence, *ANY* writes are allowed and safe. No need for any seals. Those setups existed way before `memfd_create` was added (including seals).
>
> However, when windows are resized, graphic buffers need to be resized as well. In those setups, the graphics server might call `ftruncate(2)`. If you suddenly make shmem-files sealable by default, a client can set `F_SEAL_SHRINK/GROW` and the privileged graphics server will get an error from `ftruncate(2)`, which it might not be able to handle, as it correctly never expected this to happen.
>

The graphic buffer  is a good example for shmem-files of
not-sealable-by-default. Thanks for the details.

-Jeff


> Thanks
> David