dd: remove unnecessary fcntl dependency for stdout

[kimjune01]

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

[oech3]

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

[xtqqczze]

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

[kimjune01]

You're absolutely correct. The previous version created a File from stdout's raw fd without duplicating it, which meant when the File was dropped, it would close stdout's fd and potentially lose buffered data.

I've fixed this by:

1. Changing Dest::Stdout to hold ManuallyDrop<File> instead of File
2. Adding a new Dest::FileFromStdout variant for when stdout is redirected to a seekable file
3. Both variants now guarantee stdout's file descriptor is never closed when dropped

This preserves the optimization of avoiding the fcntl syscall while maintaining soundness. All tests pass.

Thanks for catching this!

[xtqqczze]

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]

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]

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.

[xtqqczze]

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

I think the unsafe usage in ManuallyDrop::new(unsafe { File::from_raw_fd(fd) }) is reasonable. The safety invariant is straightforward and clearly documented, and this keeps the implementation much simpler than threading rustix APIs through the rest of the codebase just to avoid a small, well-contained unsafe block.

[oech3]

We can concentrate rustix to uucore too.

[oech3]

If we tried adding platforms without FD, it would be hard to support it (without std's change or flush wrapper).

[xtqqczze]

Is Windows the only platform (outside of no_std) that doesn't support file descriptors?

[oech3]

How about UEFI?

[xtqqczze]

*-unknown-uefi is effectively no_std, which coreutils doesn't support, let's discuss that in the existing issue: #10810

[oech3]

I prefer using backport crate by core member of Rust std's team or referencing nightly Rust's code.

[oech3]

What syscall does Windows call with previous code? Does it fail with common usecases? I think we can still clone on Windows.

[kimjune01]

On Windows, try_clone_to_owned() calls DuplicateHandle, that works fine in normal containers. The failure in #12157 is Unix-specific: fcntl(F_DUPFD_CLOEXEC) returning ENOSYS in restricted containers.

The Windows path still benefits from avoiding the clone though. it's an unnecessary handle duplication. The ManuallyDrop approach is simpler on both platforms: borrow the fd/handle instead of cloning it.

Re: backport crate, happy to look at that if you have a specific one in mind. The ManuallyDrop pattern here is pretty standard for borrowing file descriptors (rustix uses the same approach internally), but open to alternatives.

[kimjune01]

I think I'm gonna put it back in drafts, this one needs to cook some more

[oech3]

We need to wait maintainer's response about #12210 .

[kimjune01]

Coming back to this with answers to each thread, after testing more carefully and reading #12210.

Re soundness (xtqqczze, inline R1): addressed in 46cec8b. Dest::Stdout now holds ManuallyDrop<File>, and new_file_from_stdout no longer routes through prepare_file (which would have taken File by value and re-armed Drop); instead it builds Dest::FileFromStdout(ManuallyDrop<File>, Density) directly. grep -n "into_inner" in dd.rs returns 0 — the inner File is never extracted, all match arms auto-deref to &mut File. 85/85 unit + 109/109 integration tests pass on the branch. I also wrote a small standalone soundness test (ManuallyDrop<File>::from_raw_fd(stdout) → write → drop wrapper → write to io::stdout() again, and the two-aliases case): no double-close, no EBADF.

Re unsafe on Unix (oech3, R2 / R4): I considered the RawWriter(rustix::stdio::stdout()) sketch. It does work for the simple read/write paths, but dd also calls Seek, metadata, set_len, posix_fadvise, and a fcntl(F_SETFL) toggle inside handle_o_direct_write. Threading BorrowedFd through every arm is real surgery (and dd currently uses nix, not rustix, which adds a dep migration). The ManuallyDrop<File> pattern keeps the unsafe to one well-documented site with a straightforward invariant. Your later note (xtqqczze, R3) that "the unsafe usage in ManuallyDrop::new(unsafe { File::from_raw_fd(fd) }) is reasonable" matches my read.

Re #12210 (xtqqczze, R3): this is the strictly more general version of what 12208 does — uucore::stdio::{StdinRaw, StdoutRaw} wrapping ManuallyDrop<File>, with Deref<Target=File> so safe code can't extract and accidentally drop. Both PRs end up holding ManuallyDrop<File>, so they're structurally compatible. I'd defer to maintainers on landing order: happy to either (a) rebase 12208 on 12210 once that lands and convert to the StdoutRaw API, or (b) close 12208 in favor of 12210 if you'd rather consolidate. The case for landing 12208 as a stopgap is that #12157 has a real seccomp-restricted-container repro and 12210 is a broader refactor (cat/dd/yes + new uucore module + new rustix dep) that may take longer.

Re Windows clone (oech3, R10): confirmed — Windows uses DuplicateHandle for try_clone_to_owned, which works fine in restricted containers. The bug in #12157 is Unix-specific (fcntl F_DUPFD_CLOEXEC → ENOSYS). On the question of whether to keep the clone on Windows: the Windows path was symmetric in the original (one syscall to duplicate), and the ManuallyDrop<File>::from_raw_handle swap is symmetric here (zero syscalls). Splitting policies — borrow on Unix, clone on Windows — would mean two answers to the same question with no observable benefit on Windows. I left it symmetric because PR #12210 also takes the symmetric approach. If you'd prefer Windows-keeps-clone, easy to change; would be one cfg(windows) block reverting to OwnedFileDescriptorOrHandle::from(io::stdout())? for the Windows arm.

Re backport crate (oech3, R9): I searched and didn't find one. BorrowedFd::try_clone_to_owned in nightly std unconditionally calls libc::fcntl(_, F_DUPFD_CLOEXEC, 3) (no in-flight RFC for an alternative). rustix::stdio::take_stdout() returns OwnedFd (closest primitive, but it still requires ManuallyDrop to avoid the close on drop — which is exactly what #12210 does). If you have a specific crate in mind, please point me at it and I'll try it out.

Re no-FD platforms (R5/R6/R7/R8): I read the resolution that UEFI is no_std and out of scope per #10810. The cfg(unix) + cfg(windows) partition covers every platform coreutils currently builds for (including the freshly-landed wasm32-wasip1 from #12258, since cfg(unix) covers wasi). If a future supported platform lacks file descriptors, both this PR and #12210 would need an additional cfg branch — same forward problem for both designs.

Side note — the comment in src/uucore/src/lib/mods/io.rs:50 (// todo: remove clone introducing additional syscall dependency) suggests there's already maintainer intent to remove this at the abstraction layer, which is what #12210 does cleanly. If the preferred path is "kill this at the OwnedFileDescriptorOrHandle level," I can also explore extending that abstraction with a borrowed_from_stdout() constructor — happy to take direction on which layer you want the fix at.

I tested locally on macOS only. If the Windows symmetry is in question, I have a Windows machine and can verify the from_raw_handle path there — let me know which test would be most useful (I was thinking: dd writing to a redirected file on Windows, plus the seek=N redirected-stdout path).

Leaving in draft until the layer question is settled.

[kimjune01]

Closing in favor of #12210 — same ManuallyDrop<File> pattern, lifted into uucore::stdio so every util benefits, not just dd. Thanks @oech3 @xtqqczze for the careful review.