From 2d8a9de06537045fe7a7906edd442c0b227f43bb Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 22 Oct 2024 23:15:36 +0100 Subject: [PATCH 1/8] unistd: adding linux(glibc)/freebsd close_range. Allows to close a range of file descriptors, can set close-on-exec on these and/or unsharing (as having its own file descriptors table after fork for the calling process). --- src/unistd.rs | 46 +++++++++++++++++++++++++++++++++++++++++++++ test/test_unistd.rs | 15 +++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/unistd.rs b/src/unistd.rs index 4e35cb5b32..0e33dda1d1 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -3992,3 +3992,49 @@ pub fn chflags(path: &P, flags: FileFlag) -> Result<()> { Errno::result(res).map(drop) } } + +#[cfg(any( + all(target_os = "linux", target_env = "gnu"), + target_os = "freebsd" +))] +#[cfg(feature = "fs")] +libc_bitflags! { + /// Options for close_range() + #[cfg_attr(docsrs, doc(cfg(feature = "fs")))] + pub struct CloseRangeFlags : c_uint { + #[cfg(all(target_os = "linux", target_env = "gnu"))] + /// Unshare the file descriptors range before closing them + CLOSE_RANGE_UNSHARE; + /// Set the close-on-exec flag on the file descriptors range + CLOSE_RANGE_CLOEXEC; + } +} + +feature! { +#![feature = "fs"] + +/// Close all the file descriptor from a given range. +/// An optional flag can be applied to modify its behavior. +#[cfg(any( + all(target_os = "linux", target_env = "gnu"), + target_os = "freebsd" +))] +pub fn close_range(fdbegin: F, fdlast: F, flags: CloseRangeFlags) -> Result> { + use std::os::fd::AsRawFd; + + let raw = unsafe { + Errno::clear(); + libc::close_range(fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) + }; + if raw == -1 { + if Errno::last_raw() == 0 { + Ok(None) + } else { + Err(Errno::last()) + } + } else { + Ok(Some(raw)) + } + +} +} diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 5cb019bd95..cab4a585e8 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -1391,3 +1391,18 @@ fn test_group_from() { assert_eq!(group.gid, group_id); assert_eq!(group.name, "wheel"); } + +#[test] +#[cfg(any(all(target_os = "linux", target_env = "gnu"), target_os = "freebsd"))] +fn test_close_range() { + use tempfile::NamedTempFile; + const CONTENTS: &[u8] = b"abcdef123456"; + let mut tempfile1 = NamedTempFile::new().unwrap(); + let mut tempfile2 = NamedTempFile::new().unwrap(); + let mut tempfile3 = NamedTempFile::new().unwrap(); + tempfile3.write_all(CONTENTS).unwrap(); + tempfile2.write_all(CONTENTS).unwrap(); + tempfile1.write_all(CONTENTS).unwrap(); + let areclosed = close_range(tempfile1, tempfile3, CloseRangeFlags::CLOSE_RANGE_CLOEXEC); + assert_eq!(areclosed.expect("close_range failed").expect("invalid flag"), 0); +} From a09ca4fc8d445ca76e9c56e832287ee0ad5da290 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 22 Oct 2024 23:23:18 +0100 Subject: [PATCH 2/8] add changelog entry --- changelog/2525.added.md | 1 + test/test_unistd.rs | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 changelog/2525.added.md diff --git a/changelog/2525.added.md b/changelog/2525.added.md new file mode 100644 index 0000000000..89cd93f2e8 --- /dev/null +++ b/changelog/2525.added.md @@ -0,0 +1 @@ +Add `close_range` in unistd for Linux/glibc and FreeBSD diff --git a/test/test_unistd.rs b/test/test_unistd.rs index cab4a585e8..6ed4af5dea 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -1393,7 +1393,10 @@ fn test_group_from() { } #[test] -#[cfg(any(all(target_os = "linux", target_env = "gnu"), target_os = "freebsd"))] +#[cfg(any( + all(target_os = "linux", target_env = "gnu"), + target_os = "freebsd" +))] fn test_close_range() { use tempfile::NamedTempFile; const CONTENTS: &[u8] = b"abcdef123456"; @@ -1403,6 +1406,12 @@ fn test_close_range() { tempfile3.write_all(CONTENTS).unwrap(); tempfile2.write_all(CONTENTS).unwrap(); tempfile1.write_all(CONTENTS).unwrap(); - let areclosed = close_range(tempfile1, tempfile3, CloseRangeFlags::CLOSE_RANGE_CLOEXEC); - assert_eq!(areclosed.expect("close_range failed").expect("invalid flag"), 0); + let areclosed = + close_range(tempfile1, tempfile3, CloseRangeFlags::CLOSE_RANGE_CLOEXEC); + assert_eq!( + areclosed + .expect("close_range failed") + .expect("invalid flag"), + 0 + ); } From 78f4ca2e22b7231b423f46a41c4fa26d32134712 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 22 Oct 2024 23:40:22 +0100 Subject: [PATCH 3/8] glibc wrapper only available on recent glibc releases --- src/unistd.rs | 11 ++++++++++- test/test_unistd.rs | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/unistd.rs b/src/unistd.rs index 0e33dda1d1..b414866cdd 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -4024,7 +4024,14 @@ pub fn close_range(fdbegin: F, fdlast: F, flags: CloseRang let raw = unsafe { Errno::clear(); - libc::close_range(fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) + + cfg_if! { + if #[cfg(all(target_os = "linux", target_env = "gnu"))] { + libc::syscall(libc::SYS_close_range, fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) + } else { + libc::close_range(fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) + } + } }; if raw == -1 { if Errno::last_raw() == 0 { @@ -4033,6 +4040,8 @@ pub fn close_range(fdbegin: F, fdlast: F, flags: CloseRang Err(Errno::last()) } } else { + #[cfg(all(target_os = "linux", target_env = "gnu", target_pointer_width = "64"))] + let raw = raw as i32; Ok(Some(raw)) } diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 6ed4af5dea..61dbc812e0 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -1397,6 +1397,7 @@ fn test_group_from() { all(target_os = "linux", target_env = "gnu"), target_os = "freebsd" ))] +#[cfg_attr(qemu, ignore)] fn test_close_range() { use tempfile::NamedTempFile; const CONTENTS: &[u8] = b"abcdef123456"; From e67aeaa140282570f5317375923ea03657fe3a42 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 10 Nov 2024 16:52:08 +0000 Subject: [PATCH 4/8] update libc --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 0665795ed2..36dd6394a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ targets = [ ] [dependencies] -libc = { version = "0.2.160", features = ["extra_traits"] } +libc = { version = "0.2.162", features = ["extra_traits"] } bitflags = "2.3.3" cfg-if = "1.0" pin-utils = { version = "0.1.0", optional = true } From 7c83399bde2d1881878c053f9d87ac89a92b3f72 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 10 Nov 2024 17:04:18 +0000 Subject: [PATCH 5/8] fix tests --- test/sys/test_socket.rs | 2 +- test/test_sendfile.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index 93aa606822..a5e022dc6e 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -2914,7 +2914,7 @@ mod linux_errqueue { ) .unwrap(); // The sent message / destination associated with the error is returned: - assert_eq!(msg.bytes, MESSAGE_CONTENTS.as_bytes().len()); + assert_eq!(msg.bytes, MESSAGE_CONTENTS.len()); // recvmsg(2): "The original destination address of the datagram that caused the error is // supplied via msg_name;" however, this is not literally true. E.g., an earlier version // of this test used 0.0.0.0 (::0) as the destination address, which was mutated into diff --git a/test/test_sendfile.rs b/test/test_sendfile.rs index 3ca8092fc6..40cefbb90d 100644 --- a/test/test_sendfile.rs +++ b/test/test_sendfile.rs @@ -143,7 +143,7 @@ fn test_sendfile_dragonfly() { + &trailer_strings.concat(); // Verify the message that was sent - assert_eq!(bytes_written as usize, expected_string.as_bytes().len()); + assert_eq!(bytes_written as usize, expected_string.len()); let mut read_string = String::new(); let bytes_read = rd.read_to_string(&mut read_string).unwrap(); From a3ca8a803d018177afdf054c3292641a52b2a511 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 11 Nov 2024 06:21:35 +0000 Subject: [PATCH 6/8] address some of the concerns --- src/unistd.rs | 19 +++++++++---------- test/test_unistd.rs | 17 ++++++++++------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index b414866cdd..27ec5cb497 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -4001,12 +4001,13 @@ pub fn chflags(path: &P, flags: FileFlag) -> Result<()> { libc_bitflags! { /// Options for close_range() #[cfg_attr(docsrs, doc(cfg(feature = "fs")))] - pub struct CloseRangeFlags : c_uint { + pub struct CloseRangeFlags : c_int { #[cfg(all(target_os = "linux", target_env = "gnu"))] - /// Unshare the file descriptors range before closing them - CLOSE_RANGE_UNSHARE; - /// Set the close-on-exec flag on the file descriptors range - CLOSE_RANGE_CLOEXEC; + /// Unshare the file descriptors table, then close the file descriptors specified in the + /// range + CLOSE_RANGE_UNSHARE as c_int; + /// Set the close-on-exec flag on the file descriptors range instead of closing them + CLOSE_RANGE_CLOEXEC as c_int; } } @@ -4019,17 +4020,15 @@ feature! { all(target_os = "linux", target_env = "gnu"), target_os = "freebsd" ))] -pub fn close_range(fdbegin: F, fdlast: F, flags: CloseRangeFlags) -> Result> { - use std::os::fd::AsRawFd; - +pub unsafe fn close_range(fdbegin: F, fdlast: F, flags: CloseRangeFlags) -> Result> { let raw = unsafe { Errno::clear(); cfg_if! { if #[cfg(all(target_os = "linux", target_env = "gnu"))] { - libc::syscall(libc::SYS_close_range, fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) + libc::syscall(libc::SYS_close_range, fdbegin.as_raw_fd() as u32, fdlast.as_raw_fd() as u32, flags.bits()) } else { - libc::close_range(fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) + libc::close_range(fdbegin.as_raw_fd() as u32, fdlast.as_raw_fd() as u32, flags.bits()) } } }; diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 61dbc812e0..bd7993edd4 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -1401,14 +1401,17 @@ fn test_group_from() { fn test_close_range() { use tempfile::NamedTempFile; const CONTENTS: &[u8] = b"abcdef123456"; - let mut tempfile1 = NamedTempFile::new().unwrap(); - let mut tempfile2 = NamedTempFile::new().unwrap(); - let mut tempfile3 = NamedTempFile::new().unwrap(); - tempfile3.write_all(CONTENTS).unwrap(); - tempfile2.write_all(CONTENTS).unwrap(); - tempfile1.write_all(CONTENTS).unwrap(); + let mut tempfile: [NamedTempFile; 3] = [ + NamedTempFile::new().unwrap(), + NamedTempFile::new().unwrap(), + NamedTempFile::new().unwrap(), + ]; + + for tf in &mut tempfile { + let _ = tf.write_all(CONTENTS); + } let areclosed = - close_range(tempfile1, tempfile3, CloseRangeFlags::CLOSE_RANGE_CLOEXEC); + unsafe { close_range(tempfile[0].as_file().as_fd(), tempfile[2].as_file().as_fd(), CloseRangeFlags::CLOSE_RANGE_CLOEXEC) }; assert_eq!( areclosed .expect("close_range failed") From 6542200b3bec21c87095b72cfcd3ca5186685c79 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 11 Nov 2024 12:45:37 +0000 Subject: [PATCH 7/8] revert (unrelated) tests fixes --- test/sys/test_socket.rs | 2 +- test/test_sendfile.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index a5e022dc6e..93aa606822 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -2914,7 +2914,7 @@ mod linux_errqueue { ) .unwrap(); // The sent message / destination associated with the error is returned: - assert_eq!(msg.bytes, MESSAGE_CONTENTS.len()); + assert_eq!(msg.bytes, MESSAGE_CONTENTS.as_bytes().len()); // recvmsg(2): "The original destination address of the datagram that caused the error is // supplied via msg_name;" however, this is not literally true. E.g., an earlier version // of this test used 0.0.0.0 (::0) as the destination address, which was mutated into diff --git a/test/test_sendfile.rs b/test/test_sendfile.rs index 40cefbb90d..3ca8092fc6 100644 --- a/test/test_sendfile.rs +++ b/test/test_sendfile.rs @@ -143,7 +143,7 @@ fn test_sendfile_dragonfly() { + &trailer_strings.concat(); // Verify the message that was sent - assert_eq!(bytes_written as usize, expected_string.len()); + assert_eq!(bytes_written as usize, expected_string.as_bytes().len()); let mut read_string = String::new(); let bytes_read = rd.read_to_string(&mut read_string).unwrap(); From bc14f9540507b15d615d6b2d254408ecf96faf95 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 11 Nov 2024 12:48:47 +0000 Subject: [PATCH 8/8] doc addition --- src/unistd.rs | 4 ++++ test/test_unistd.rs | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index 27ec5cb497..d2baa0ce2c 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -4016,6 +4016,10 @@ feature! { /// Close all the file descriptor from a given range. /// An optional flag can be applied to modify its behavior. +/// +/// # Safety +/// +/// This function as there are risks of double closes on the file descriptors. #[cfg(any( all(target_os = "linux", target_env = "gnu"), target_os = "freebsd" diff --git a/test/test_unistd.rs b/test/test_unistd.rs index bd7993edd4..f3c961cfef 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -1410,8 +1410,13 @@ fn test_close_range() { for tf in &mut tempfile { let _ = tf.write_all(CONTENTS); } - let areclosed = - unsafe { close_range(tempfile[0].as_file().as_fd(), tempfile[2].as_file().as_fd(), CloseRangeFlags::CLOSE_RANGE_CLOEXEC) }; + let areclosed = unsafe { + close_range( + tempfile[0].as_file().as_fd(), + tempfile[2].as_file().as_fd(), + CloseRangeFlags::CLOSE_RANGE_CLOEXEC, + ) + }; assert_eq!( areclosed .expect("close_range failed")