tempfile: Add support for TempFile

[cgwalters]

The cap-tempfile crate ironically only supports temporary
directories right now.

This adapts code I wrote as part of https://github.com/coreos/cap-std-ext/
to add support for temporary files that can also be atomically
linked into place.

[cgwalters]

Side note...I have kind of lost track of how many times I have implemented Linux O_TMPFILE stuff. A long time ago I wrote a C implementation of this in ostree, then cleaned it up and factored it out into https://gitlab.gnome.org/GNOME/libglnx where I also did fd-relative i.e. C openat() stuff (but still not capability/openat2 based).

Then...Rust came along, I eventually decided to build on openat and reimplement it again in openat-ext.

So this is at least the 4th time, but I am pretty sure I also did some stuff around this in glib but at this point that's getting to be a distant memory.

But anyways, I for one am looking forward to going into the second half of my career mostly writing Rust code, and also hopefully not writing basic code to manipulate temporary files again...

[cgwalters]

OK I belatedly realized that having the API pass in Permissions was really just a workaround for a long ago fixed glibc bug. I think we can relatively safely rely on that being fixed - if some user shows up bitten by it, we could have rustix do the direct syscall path, or perhaps try to detect it and fall back to not using O_TMPFILE.

(The latter path will want the "compute current process umask" code that lives in the test suite for now)

[sunfishcode]

OK I belatedly realized that having the API pass in Permissions was really just a workaround for a long ago fixed glibc bug. I think we can relatively safely rely on that being fixed - if some user shows up bitten by it, we could have rustix do the direct syscall path, or perhaps try to detect it and fall back to not using O_TMPFILE.

According to the linked bug report, the bug is present in glibc 2.20, however Rust itself is only now updating its minimum glibc version to 2.17. Unless it's a major obstacle, I'd prefer to have cap-std continue to support the versions of glibc supported by Rust.

[cgwalters]

OK, MacOS and Windows should compile at least now.

[cgwalters]

According to the linked bug report, the bug is present in glibc 2.20, however Rust itself is only now rust-lang/rust#95026. Unless it's a major obstacle, I'd prefer to have cap-std continue to support the versions of glibc supported by Rust.

Yeah, fair. Though it should be noted, it looks like the tempfile crate uses this unconditionally today.

(We need to match the same errno fallbacks too, will look at that) done

Perhaps one possible trick here could be to use a weak symbol to detect if we have a new enough glibc? I'd guess rustix could expose some API for this, given that it's effectively doing it internally; for example if we have getrandom we know we have a new enough glibc.

[cgwalters]

Perhaps one possible trick here could be to use a weak symbol to detect if we have a new enough glibc?

Or we could use openat2 - we already have all the infrastructure for that. The downside though is this would exclude systems that have new enough Linux for O_TMPFILE but not openat2. But OTOH people using cap-std on such old systems are already going to be paying a high price and this would only be another small hit.

[cgwalters]

Or we could use openat2

I think to do that nicely we'd want to make open_beneath from cap-primitives be pub or so? There are important things like handling android and seccomp there.

[sunfishcode]

(Don't worry about the windows-2016 failure btw.)

I think this PR is in good shape; I just want to figure out the O_TMPFILE situation here.

According to the linked bug report, the bug is present in glibc 2.20, however Rust itself is only now rust-lang/rust#95026. Unless it's a major obstacle, I'd prefer to have cap-std continue to support the versions of glibc supported by Rust.

Yeah, fair. Though it should be noted, it looks like the tempfile crate uses this unconditionally today.

If I understand correctly, that code in the tempfile is using open rather than openat, so on x86_64 and possibly others the bug is hidden in practice (according to the glibc bug linked above).

Perhaps one possible trick here could be to use a weak symbol to detect if we have a new enough glibc? I'd guess rustix could expose some API for this, given that it's effectively doing it internally; for example if we have getrandom we know we have a new enough glibc.

My sense here is that this is something that would be better to handle internally in rustix, rather than exposing an API. Detecting it by using the weak! macro looking for getrandom sounds reasonable to me, and for a fallback when we don't have a new enough glibc, rustix could use libc::syscall(SYS_openat, ...) , which avoids the bug. Does that sound workable?

[cgwalters]

If I understand correctly, that code in the tempfile is using open rather than openat, so on x86_64 and possibly others the bug is hidden in practice (according to the glibc bug linked above).

Ah yes, I think you're right.

My sense here is that this is something that would be better to handle internally in rustix, rather than exposing an API. Detecting it by using the weak! macro looking for getrandom sounds reasonable to me, and for a fallback when we don't have a new enough glibc, rustix could use libc::syscall(SYS_openat, ...) , which avoids the bug. Does that sound workable?

Sounds good to me!

[cgwalters]

Detecting it by using the weak! macro looking for getrandom sounds reasonable to me

I briefly looked at this, and today the weak! is wrapped up in weak_or_syscall!, and I couldn't figure out an elegant way to make the weak! inside there conditionally pub(crate) or so.

Here's my hack to duplicate it:

From 84dfe8d0600a39c907a887ae7237a5f24a6410d5 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 1 Apr 2022 08:45:09 -0400
Subject: [PATCH] wip

---
 src/imp/libc/mod.rs           | 6 ++++++
 src/imp/libc/rand/syscalls.rs | 3 +++
 src/imp/libc/weak.rs          | 4 ++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/imp/libc/mod.rs b/src/imp/libc/mod.rs
index 96a040b..1fbcbbd 100644
--- a/src/imp/libc/mod.rs
+++ b/src/imp/libc/mod.rs
@@ -62,3 +62,9 @@ pub(crate) mod rand;
 pub(crate) mod thread;
 #[cfg(not(windows))]
 pub(crate) mod time;
+
+/// If the host libc is glibc, detect if it's at least version 2.25.
+#[cfg(not(windows))]
+pub(crate) fn is_atleast_glibc_2_25() -> bool {
+    rand::syscalls::libc_getrandom_weak.get().is_some()
+}
diff --git a/src/imp/libc/rand/syscalls.rs b/src/imp/libc/rand/syscalls.rs
index e860f40..380f643 100644
--- a/src/imp/libc/rand/syscalls.rs
+++ b/src/imp/libc/rand/syscalls.rs
@@ -5,6 +5,9 @@
 #[cfg(target_os = "linux")]
 use {super::super::c, super::super::conv::ret_ssize_t, crate::io, crate::rand::GetRandomFlags};
 
+// A duplicate weak symbol because we use this to detect the glibc version.
+weak! { pub(crate) fn libc_getrandom_weak(*mut c::c_void, c::size_t, c::c_uint) -> c::ssize_t }
+
 #[cfg(target_os = "linux")]
 pub(crate) fn getrandom(buf: &mut [u8], flags: GetRandomFlags) -> io::Result<usize> {
     // `getrandom` wasn't supported in glibc until 2.25.
diff --git a/src/imp/libc/weak.rs b/src/imp/libc/weak.rs
index aecc081..203969f 100644
--- a/src/imp/libc/weak.rs
+++ b/src/imp/libc/weak.rs
@@ -36,9 +36,9 @@ const NULL: *mut c_void = null_mut();
 const INVALID: *mut c_void = 1 as *mut c_void;
 
 macro_rules! weak {
-    (fn $name:ident($($t:ty),*) -> $ret:ty) => (
+    ($vis:vis fn $name:ident($($t:ty),*) -> $ret:ty) => (
         #[allow(non_upper_case_globals)]
-        static $name: $crate::imp::weak::Weak<unsafe extern fn($($t),*) -> $ret> =
+        $vis static $name: $crate::imp::weak::Weak<unsafe extern fn($($t),*) -> $ret> =
             $crate::imp::weak::Weak::new(concat!(stringify!($name), '\0'));
     )
 }
-- 
2.35.1

[cgwalters]

OK moving that bit to bytecodealliance/rustix#268

[sunfishcode]

bytecodealliance/rustix#268 is now released, in rustix 0.33.6.

[cgwalters]

Anything else that needs doing from my side for this?

[sunfishcode]

Ah, thanks for the ping. Looks like this is ready to go, the CI failure is just windows-2016 which is fixed on main.

[cgwalters]

Follow up to this in coreos/cap-std-ext#12