Skip to content

work around linux not honoring write_at for O_APPEND files #143166

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,23 @@ fn file_test_io_read_write_at() {
check!(fs::remove_file(&filename));
}

#[test]
#[cfg(unix)]
fn file_test_append_write_at() {
use crate::os::unix::fs::FileExt;

let tmpdir = tmpdir();
let filename = tmpdir.join("file_test_append_write_at.txt");
let msg = b"it's not working!";
check!(fs::write(&filename, &msg));
// write_at should work even in in append mode
let f = check!(fs::File::options().append(true).open(&filename));
assert_eq!(check!(f.write_at(b" ", 5)), 3);

let content = check!(fs::read(&filename));
assert_eq!(&content, b"it's working!");
}

#[test]
#[cfg(unix)]
fn set_get_unix_permissions() {
Expand Down
5 changes: 3 additions & 2 deletions library/std/src/os/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ pub trait FileExt {
///
/// # Bug
/// On some systems, `write_at` utilises [`pwrite64`] to write to files.
/// However, this syscall has a [bug] where files opened with the `O_APPEND`
/// However, on linux this syscall has a [bug] where files opened with the `O_APPEND`
/// flag fail to respect the offset parameter, always appending to the end
/// of the file instead.
/// of the file instead. When available `pwritev2(..., RWF_NOAPPEND)` will
/// be used instead to avoid this bug.
///
/// It is possible to inadvertently set this flag, like in the example below.
/// Therefore, it is important to be vigilant while changing options to mitigate
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/os/unix/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ fn write_vectored_at() {
file.write_all(msg).unwrap();
}
let expected = {
let file = fs::File::options().write(true).open(&filename).unwrap();
// Open in append mode to test that positioned writes bypass O_APPEND.
let file = fs::File::options().append(true).open(&filename).unwrap();
let buf0 = b" ";
let buf1 = b"great ";

Expand Down
82 changes: 80 additions & 2 deletions library/std/src/sys/fd/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ use crate::cmp;
use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, Read};
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd};
use crate::sys::cvt;
#[cfg(all(target_os = "android", target_pointer_width = "64"))]
#[cfg(all(any(target_os = "android", target_os = "linux"), target_pointer_width = "64"))]
use crate::sys::pal::weak::syscall;
#[cfg(any(all(target_os = "android", target_pointer_width = "32"), target_vendor = "apple"))]
#[cfg(any(
all(any(target_os = "android", target_os = "linux"), target_pointer_width = "32"),
target_vendor = "apple"
))]
use crate::sys::pal::weak::weak;
use crate::sys_common::{AsInner, FromInner, IntoInner};

Expand Down Expand Up @@ -384,6 +387,16 @@ impl FileDesc {
))]
use libc::pwrite64;

// Work around linux deviating from POSIX where it ignores the
// offset of pwrite when the file was opened with O_APPEND.
#[cfg(any(target_os = "linux", target_os = "android"))]
{
let iov = [IoSlice::new(buf)];
if let Some(ret) = self.pwritev2(&iov, offset) {
return ret;
}
}

unsafe {
cvt(pwrite64(
self.as_raw_fd(),
Expand All @@ -408,6 +421,13 @@ impl FileDesc {
target_os = "openbsd", // OpenBSD 2.7
))]
pub fn write_vectored_at(&self, bufs: &[IoSlice<'_>], offset: u64) -> io::Result<usize> {
// Work around linux deviating from POSIX where it ignores the
// offset of pwrite when the file was opened with O_APPEND.
#[cfg(any(target_os = "linux", target_os = "android"))]
if let Some(ret) = self.pwritev2(bufs, offset) {
return ret;
}

let ret = cvt(unsafe {
libc::pwritev(
self.as_raw_fd(),
Expand Down Expand Up @@ -611,6 +631,64 @@ impl FileDesc {
pub fn duplicate(&self) -> io::Result<FileDesc> {
Ok(Self(self.0.try_clone()?))
}

#[cfg(any(target_os = "linux", target_os = "android"))]
fn pwritev2(&self, bufs: &[IoSlice<'_>], offset: u64) -> Option<io::Result<usize>> {
#[cfg(target_pointer_width = "64")]
syscall!(
fn pwritev2(
fd: libc::c_int,
iovec: *const libc::iovec,
n_iovec: libc::c_int,
offset: off64_t,
flags: libc::c_int,
) -> isize;
);
#[cfg(target_pointer_width = "32")]
let pwritev2 = {
weak!(
fn pwritev2(
fd: libc::c_int,
iovec: *const libc::iovec,
n_iovec: libc::c_int,
offset: off64_t,
flags: libc::c_int,
) -> isize;
);
let Some(pwritev2) = pwritev2.get() else {
return None;
};
pwritev2
};

use core::sync::atomic::AtomicBool;

static NOAPPEND_SUPPORTED: AtomicBool = AtomicBool::new(true);
if NOAPPEND_SUPPORTED.load(core::sync::atomic::Ordering::Relaxed) {
let r = unsafe {
cvt(pwritev2(
self.as_raw_fd(),
bufs.as_ptr() as *const libc::iovec,
cmp::min(bufs.len(), max_iov()) as libc::c_int,
offset as off64_t,
libc::RWF_NOAPPEND,
))
};
match r {
Ok(ret) => return Some(Ok(ret as usize)),
Err(e)
if let Some(err) = e.raw_os_error()
&& (err == libc::EOPNOTSUPP || err == libc::ENOSYS) =>
{
NOAPPEND_SUPPORTED.store(false, core::sync::atomic::Ordering::Relaxed);
return None;
}
Err(e) => return Some(Err(e)),
}
}

return None;
}
}

impl<'a> Read for &'a FileDesc {
Expand Down
Loading