Skip to content

dd: remove unnecessary fcntl dependency for stdout#12208

Open
kimjune01 wants to merge 1 commit intouutils:mainfrom
kimjune01:fix-dd-fcntl-dependency
Open

dd: remove unnecessary fcntl dependency for stdout#12208
kimjune01 wants to merge 1 commit intouutils:mainfrom
kimjune01:fix-dd-fcntl-dependency

Conversation

@kimjune01
Copy link
Copy Markdown

Summary

dd fails in restricted containers where fcntl returns ENOSYS because try_clone_to_owned() internally calls fcntl. This was introduced in PR #10235.

  • Replaces OwnedFileDescriptorOrHandle::from(io::stdout()) with a ManuallyDrop<File> wrapper that borrows the fd without cloning
  • Eliminates the unnecessary fcntl syscall
  • Works on both Unix (from_raw_fd) and Windows (from_raw_handle)

Test plan

  • All 85 unit tests pass
  • Basic functionality verified (echo "test" | dd bs=1)
  • CI passes

Fixes #12157

Replace OwnedFileDescriptorOrHandle::from() with borrowed File pattern
using ManuallyDrop and from_raw_fd/from_raw_handle. This eliminates the
unnecessary fcntl syscall from try_clone_to_owned() when dd writes to
stdout, fixing failures in restricted containers where fcntl is
unavailable.

Fixes uutils#12157
@oech3
Copy link
Copy Markdown
Contributor

oech3 commented May 10, 2026

Can we do same things without unsafe at least on unix?

Comment thread src/uu/dd/src/dd.rs
let file = ManuallyDrop::new(unsafe { File::from_raw_handle(io::stdout().as_raw_handle()) });

Self::prepare_file(fx.into_file(), settings)
Self::prepare_file(ManuallyDrop::into_inner(file), settings)
Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is potentially unsound, we must guarantee stdout is never dropped.

@xtqqczze
Copy link
Copy Markdown
Contributor

Can we do same things without unsafe at least on unix?

I don't think so, but we can use a safe wrapper: #12210

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented May 10, 2026

rustix::io::{read,write} does not require unsafe. So it should be able to make safe wrapper at least for unix and wasi without any unsafe.

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented May 10, 2026

struct RawReader<'a>(rustix::fd::BorrowedFd<'a>);
impl io::Read for RawReader<'_> {
    fn read(&mut self, b: &mut [u8]) -> io::Result<usize> {
        rustix::io::read(self.0, b).map_err(Into::into)
    }
}
let stdin = RawReader(rustix::stdio::stdin());

[Edit] this spreads <'a> to anywhere. So RawReader(std::io::stdin()) should be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dd incorrectly depends on fcntl syscall

3 participants