On Fri, May 24, 2024 at 7:29 AM David Rheinsberg <david@xxxxxxxxxxxx> wrote: > > Hi > > On Thu, May 23, 2024, at 6:55 PM, Jeff Xu wrote: > > On Thu, May 23, 2024 at 9:20 AM Jeff Xu <jeffxu@xxxxxxxxxx> wrote: > >> On Thu, May 23, 2024 at 1:24 AM David Rheinsberg <david@xxxxxxxxxxxx> wrote: > >> > 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. > >> > > > memfd is by default not sealable, and file is by default sealable, > > right ? that makes the memfd semantics different from other objects > > in linux. > > I wonder what is the original reason to have memfd this way? > > shmem-files are *not* sealable by default. This design was followed for backward compatibility reasons, since shmem-files predate sealing and silently enabling sealing on all shmem-files would have broken existing users (see shmem.c which initializes seals to F_SEAL_SEAL). > One may ask the question: If shmem-files need to be non-sealable by default, does memfd need to be so as well? > I am not sure what you mean with "makes [memfd] semantics different from other objects in linux". Can you elaborate? > The memory sealing feature - mseal() went through similar discussion on MAP_SEALABLE flag during the RFC phase, but everyone doesn't like the flag, and it gets dropped. The feedback from communities for MAP_SEALABLE were. - such a flag will slow down the adoption of the feature, i.e. applications on multiple layers/libraries must change in order to use sealing, i.e. time of construction and time of sealing might reside in different libraries. - Deny of service attack is likely not a concern, the attacker that is able to call mseal() can probably already call mprotect() or other calls and achieve a similar DOS attack. > Since `memfd_create` was introduced at the same time as shmem-sealing, it could certainly have enabled sealing by default. Not sure whether this would be preferable, though. > I would think making memfd sealable is desirable. Probably the same for a shmem-file too. > > 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? 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. If the other party needs to write to shmem/memfd anyway, then maybe F_SEAL_EXEC needs to be applied ? Thanks -Jeff > Thanks > David
