Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use memfd instead of temporary files #14

Open
sp3d opened this issue Feb 7, 2017 · 9 comments
Open

Use memfd instead of temporary files #14

sp3d opened this issue Feb 7, 2017 · 9 comments

Comments

@sp3d
Copy link

sp3d commented Feb 7, 2017

Temporary files are essentially a hack. For the purposes of shm, there's no need to have any filesystem entry, and it's possible for the tmp directory to be disk-backed or otherwise persistent. Linux (which is the only platform supporting Wayland properly at present) provides memfd which is generally agreed upon to be the right way to get an fd referencing a memory region. Memfd is also useful for compositors to ensure they can use mapped memory without worrying about the mapping being shrunk out from under them by buggy or hostile applications; see https://lwn.net/Articles/591108/.

Avoiding the tempfile crate will also shrink the transitive dependency graph significantly.

@elinorbgr
Copy link
Member

Is there an already existing rust wrapper around memfd, or would that mean handling the syscall manually in wayland-window?

@sp3d
Copy link
Author

sp3d commented Feb 7, 2017

I don't see anything on crates.io, but there seems to be https://github.com/sdroege/memfd-rs (which looks like it presents a reasonable safe abstraction around memfd in general). Maybe ask the author if there's anything blocking a published release.

@elinorbgr
Copy link
Member

Okay, I've been thinking a little about this, and there are some things that concerns me:

  • First of all, is it linux-only ? It looks like there is some effort to port wayland to freebsd. On the other hand, there can be a compilation switch depending on the platform.
  • This will require some changes to memfd-rs, as what we really want is a memfd with only the flag F_SEAL_SHRINK set
  • Actually, the compositors will still need to support tempfile-backed SHM anyway, as this is even what official examples do

@sp3d
Copy link
Author

sp3d commented Feb 8, 2017

The memfd syscall is Linux-only. Wayland is Linux-first but should fundamentally be implementable on any OS, and I'm happy to see FreeBSD support moving along. From a cursory look, it seems that the FreeBSD equivalent of memfd is the FreeBSD-specific SHM_ANON flag for shm_open (though I'm not sure if there's any analogue for sealing).

For simple usage, it may be easiest to just use nix::sys::memfd::* directly.

Compositors are in the position of needing to support the sloppiest-implemented client users will want to run; most will support tempfile-backed shm as well as a million other features (e.g. X11 compatibility), but the beauty of the Wayland protocol is that it doesn't require all that complexity, and use-case-specific compositors can be much stricter and simpler than the DEs that most users run on their desktops. Clients can be made simpler and more robust by avoiding dependence on the filesystem (even open() with the Linux O_TMPFILE option involves creating an inode in the underlying filesystem which isn't guaranteed to be tmpfs) for their shared-memory mappings.

Of course this isn't a high-priority bug, since user-facing functionality shouldn't change noticeably, but it would be a nice change that reduces the total number of moving parts in the stack as a whole.

@valpackett
Copy link

Yep, SHM_ANON is the way to go on FreeBSD, I recently added that constant to rust libc and added its usage to ipc-channel (waiting for a libc release). Maybe we need to extract that create_shmem code into a new crate.

What does sealing mean?

@valpackett
Copy link

Published the tiny memfd/SHM_ANON wrapper crate! https://github.com/myfreeweb/shmemfdrs

@elinorbgr
Copy link
Member

@myfreeweb that's an interesting crate, thanks for notifying!

Just to make sure is the returned c_int from it a regular file descriptor, that can be given to File::from_raw_fd(..) and used like a regular rust File?

If so, would it make sense to integrate this casting in your API, making it usable with a safe interface?

@valpackett
Copy link

It was extracted from Servo's ipc-channel and they use a raw c_int… I didn't want to change their code much.

@sp3d
Copy link
Author

sp3d commented Dec 25, 2017

@myfreeweb, sealing is well-described at https://dvdhrm.wordpress.com/tag/memfd/. For Wayland IPC the main use-case is SEAL_SHRINK to prevent clients from causing bus errors in the server when operating on mmap'd regions that the client shrinks out from under the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants