Skip to content

Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND #82417

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

Merged
merged 2 commits into from
Mar 12, 2021
Merged
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
18 changes: 10 additions & 8 deletions library/std/src/sys/unix/kernel_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use crate::process::{ChildStderr, ChildStdin, ChildStdout};
use crate::ptr;
use crate::sync::atomic::{AtomicBool, AtomicU8, Ordering};
use crate::sys::cvt;
use libc::{EBADF, EINVAL, ENOSYS, EOPNOTSUPP, EOVERFLOW, EPERM, EXDEV};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -535,7 +536,7 @@ pub(super) fn copy_regular_files(reader: RawFd, writer: RawFd, max_len: u64) ->
cvt(copy_file_range(INVALID_FD, ptr::null_mut(), INVALID_FD, ptr::null_mut(), 1, 0))
};

if matches!(result.map_err(|e| e.raw_os_error()), Err(Some(libc::EBADF))) {
if matches!(result.map_err(|e| e.raw_os_error()), Err(Some(EBADF))) {
HAS_COPY_FILE_RANGE.store(AVAILABLE, Ordering::Relaxed);
} else {
HAS_COPY_FILE_RANGE.store(UNAVAILABLE, Ordering::Relaxed);
Expand Down Expand Up @@ -573,19 +574,20 @@ pub(super) fn copy_regular_files(reader: RawFd, writer: RawFd, max_len: u64) ->
Err(err) => {
return match err.raw_os_error() {
// when file offset + max_length > u64::MAX
Some(libc::EOVERFLOW) => CopyResult::Fallback(written),
Some(
libc::ENOSYS | libc::EXDEV | libc::EINVAL | libc::EPERM | libc::EOPNOTSUPP,
) => {
Some(EOVERFLOW) => CopyResult::Fallback(written),
Some(ENOSYS | EXDEV | EINVAL | EPERM | EOPNOTSUPP | EBADF) => {
// Try fallback io::copy if either:
// - Kernel version is < 4.5 (ENOSYS¹)
// - Files are mounted on different fs (EXDEV)
// - copy_file_range is broken in various ways on RHEL/CentOS 7 (EOPNOTSUPP)
// - copy_file_range file is immutable or syscall is blocked by seccomp¹ (EPERM)
// - copy_file_range cannot be used with pipes or device nodes (EINVAL)
// - the writer fd was opened with O_APPEND (EBADF²)
//
// ¹ these cases should be detected by the initial probe but we handle them here
// anyway in case syscall interception changes during runtime
// ² actually invalid file descriptors would cause this too, but in that case
// the fallback code path is expected to encounter the same error again
assert_eq!(written, 0);
CopyResult::Fallback(0)
}
Expand Down Expand Up @@ -649,7 +651,7 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) ->
Ok(ret) => written += ret as u64,
Err(err) => {
return match err.raw_os_error() {
Some(libc::ENOSYS | libc::EPERM) => {
Some(ENOSYS | EPERM) => {
// syscall not supported (ENOSYS)
// syscall is disallowed, e.g. by seccomp (EPERM)
match mode {
Expand All @@ -659,12 +661,12 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) ->
assert_eq!(written, 0);
CopyResult::Fallback(0)
}
Some(libc::EINVAL) => {
Some(EINVAL) => {
// splice/sendfile do not support this particular file descriptor (EINVAL)
assert_eq!(written, 0);
CopyResult::Fallback(0)
}
Some(os_err) if mode == SpliceMode::Sendfile && os_err == libc::EOVERFLOW => {
Some(os_err) if mode == SpliceMode::Sendfile && os_err == EOVERFLOW => {
CopyResult::Fallback(written)
}
_ => CopyResult::Error(err, written),
Expand Down
18 changes: 18 additions & 0 deletions library/std/src/sys/unix/kernel_copy/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@ fn copy_specialization() -> Result<()> {
result.and(rm1).and(rm2)
}

#[test]
fn copies_append_mode_sink() -> Result<()> {
let tmp_path = tmpdir();
let source_path = tmp_path.join("copies_append_mode.source");
let sink_path = tmp_path.join("copies_append_mode.sink");
let mut source =
OpenOptions::new().create(true).truncate(true).write(true).read(true).open(&source_path)?;
write!(source, "not empty")?;
source.seek(SeekFrom::Start(0))?;
let mut sink = OpenOptions::new().create(true).append(true).open(&sink_path)?;

let copied = crate::io::copy(&mut source, &mut sink)?;

assert_eq!(copied, 9);

Ok(())
}

#[bench]
fn bench_file_to_file_copy(b: &mut test::Bencher) {
const BYTES: usize = 128 * 1024;
Expand Down