Received-SPF: pass (google.com: domain of linux-kernel+bounces-206439-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240513191544.94754-1-pobrn@protonmail.com> <CALmYWFt7MYbWrCDVEKH4DrMQGxaXA2kK8qth-JVxzkvMd6Ohtg@mail.gmail.com>
 <20240522162324.0aeba086228eddd8aff4f628@linux-foundation.org>
 <1KDsEBw8g7ymBVpGJZp9NRH1HmCBsQ_jjQ_jKOg90gLUFhW5W6lcG-bI4-5OPkrD24RiG7G83VoZL4SXPQjfldsNFDg7bFnFFgrVZWwSWXQ=@protonmail.com>
 <08450f80-4c33-40db-886f-fee18e531545@app.fastmail.com> <CALmYWFv9dK5ZPzwx3WCLMXzuuDadvFxh84+8rrT7aL105+ZZAQ@mail.gmail.com>
 <CALmYWFtedtEnfGFp5DYacHYOE7+GB8yoQC-iyw7JAxySmgQ7vw@mail.gmail.com>
 <f880562e-9521-4270-82e2-c6fb14dc853a@app.fastmail.com> <CALmYWFuPBEM2DE97mQvB2eEgSO9Dvt=uO9OewMhGfhGCY66Hbw@mail.gmail.com>
 <1e1edbdc-f91f-4106-baa6-b765b78e6abc@app.fastmail.com>
In-Reply-To: <1e1edbdc-f91f-4106-baa6-b765b78e6abc@app.fastmail.com>
From: Jeff Xu <jeffxu@chromium.org>
Date: Fri, 7 Jun 2024 08:58:08 -0700
Message-ID: <CABi2SkUMppyL_LRKJV6BfgGu=1GpGCEOdZ5VHCENMUtzHcRTkA@mail.gmail.com>
Subject: Re: [PATCH v1] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING`
To: David Rheinsberg <david@readahead.eu>
Cc: Jeff Xu <jeffxu@google.com>, Aleksa Sarai <cyphar@cyphar.com>, 
	=?UTF-8?B?QmFybmFiw6FzIFDFkWN6ZQ==?= <pobrn@protonmail.com>, 
	Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, 
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, 
	dmitry.torokhov@gmail.com, Daniel Verkamp <dverkamp@chromium.org>, hughd@google.com, 
	jorgelo@chromium.org, skhan@linuxfoundation.org, 
	Kees Cook <keescook@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi David,

On Fri, Jun 7, 2024 at 1:38=E2=80=AFAM David Rheinsberg <david@readahead.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 b=
e
> >> > ?
> >> > 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 existin=
g shmem-users that share file-descriptors and *expect* the other party to b=
e 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 acce=
ss to the buffer (e.g., render-client shares a buffer with the display serv=
er for scanout), so just because you can *write* to a shmem-file does not m=
ean that sharing is unsafe (e.g., using SIGBUS+mmap can safely deal with pa=
ge-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 clie=
nt is free to write garbage into the buffer. This is not unsafe. The graphi=
cs 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_cr=
eate` 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 yo=
u suddenly make shmem-files sealable by default, a client can set `F_SEAL_S=
HRINK/GROW` and the privileged graphics server will get an error from `ftru=
ncate(2)`, which it might not be able to handle, as it correctly never expe=
cted 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