Skip to content

Commit

Permalink
Auto merge of #82417 - the8472:fix-copy_file_range-append, r=m-ou-se
Browse files Browse the repository at this point in the history
Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND

fixes #82410

While `sendfile()` returns `EINVAL` when the output was opened with O_APPEND,  `copy_file_range()` does not and returns `EBADF` instead, which – unlike other `EBADF` causes – is not fatal for this operation since a regular `write()` will likely succeed.

We now treat `EBADF` as a non-fatal error for `copy_file_range` and fall back to a read-write copy as we already did for several other errors.
  • Loading branch information
bors committed Mar 11, 2021
2 parents 4a8b6f7 + 81602fb commit 03e864f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
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

0 comments on commit 03e864f

Please sign in to comment.