I/O safety. #87329

sunfishcode · 2021-07-21T00:39:35Z

Introduce OwnedFd and BorrowedFd, and the AsFd trait, and
implementations of AsFd, From<OwnedFd> and From<T> for OwnedFd
for relevant types, along with Windows counterparts for handles and
sockets.

Tracking issue: #87074

RFC: https://github.com/rust-lang/rfcs/blob/master/text/3128-io-safety.md

Highlights:

The doc comments at the top of library/std/src/os/unix/io/mod.rs and library/std/src/os/windows/io/mod.rs
The new types and traits in library/std/src/os/unix/io/fd.rs and library/std/src/os/windows/io/handle.rs
The removal of the RawHandle struct the Windows impl, which had the same name as the RawHandle type alias, and its functionality is now folded into Handle.

Managing five levels of wrapping (File wraps sys::fs::File wraps sys::fs::FileDesc wraps OwnedFd wraps RawFd, etc.) made for a fair amount of churn and verbose as/into/from sequences in some places. I've managed to simplify some of them, but I'm open to ideas here.

r? @joshtriplett

joshtriplett · 2021-07-21T05:08:22Z

Managing five levels of wrapping (File wraps sys::fs::File wraps sys::fs::FileDesc wraps OwnedFd wraps RawFd, etc.) made for a fair amount of churn and verbose as/into/from sequences in some places. I've managed to simplify some of them, but I'm open to ideas here.

I don't know how feasible this would be, but I wonder if we could eliminate sys::fs::FileDesc and have sys::fs::File directly use OwnedFd. That would give us what feels like the right amount of indirection: the portable File wraps the UNIX-specific sys::fs::File, which wraps an OwnedFd, which wraps the simple type alias RawFd.

(To clarify, though, that kind of cleanup belongs in a follow-up, not in the initial introduction of this.)

library/std/src/os/unix/io/fd.rs

library/std/src/os/wasi/io/fd.rs

library/std/src/os/windows/io/handle.rs

library/std/src/os/unix/io/fd.rs

library/std/src/os/windows/io/socket.rs

Thomasdezeeuw

Looks really good @sunfishcode, just some small comments.

library/std/src/os/unix/io/mod.rs

library/std/src/os/wasi/io/raw.rs

library/std/src/os/windows/io/handle.rs

Thomasdezeeuw · 2021-07-21T07:46:17Z

library/std/src/os/windows/io/handle.rs

+/// so it can be used in FFI in places where a handle is passed as a consumed
+/// argument or returned as an owned value, and is never null.
+///
+/// Note that it *may* have the value `INVALID_HANDLE_VALUE`. See [here] for


Same as about with regard to INVALID_HANDLE_VALUE.

library/std/src/os/windows/io/handle.rs

library/std/src/os/windows/io/mod.rs

ChrisDenton · 2021-07-21T21:15:58Z

In case it's relevant here, Windows essentially has two types of I/O HANDLE (it also reuses the type for other things but that's off topic):

Kernel handles. These are "real" handles (i.e. they are an offset into a table of kernel objects).
Pseudo handles. These are "fake" handles (i.e. managed by the Win32 subsystem). They have resources managed outside of the NT kernel so are often intentionally invalid kernel handles. The meaning of pseudo handles is very context dependent.

In both cases, zero is not a valid handle. It's also guaranteed that a kernel handle will not have the lowest two bits set. A pseudo handle has no such restriction and indeed may intentionally make use of the lowest two bits to distinguish themselves from "real" handles. Therefore INVALID_HANDLE_VALUE is not a valid kernel handle but can be reused as a pseudo handle.

(I'm also pretty sure a kernel handle will always be less than i32::MAX but I'm not sure to what extent that's guaranteed).

I'm uncertain if this information is helpful to anyone but I just wanted to document some of what I know.

sunfishcode · 2021-07-21T21:37:48Z

Thanks @ChrisDenton! Do you by chance know of an authoritative reference stating that zero is not a valid handle? The code here assumes that, and for various reasons it seems like a safe assumption, but if there's official documentation about this, I'd like to cite it.

ChrisDenton · 2021-07-21T21:46:30Z

Unfortunately I do not. It shakes out that way, as you note, for various practical reasons but I don't know that it's explicitly documented anywhere.

ChrisDenton · 2021-07-22T12:46:19Z

@sunfishcode From GetStdHandle:

If an application does not have associated standard handles, such as a service running on an interactive desktop, and has not redirected them, the return value is NULL.

So here a NULL I/O handle means "no handle". You can redirect stdin etc to any I/O handle (where "I/O handle" means "can be passed to WriteFile, etc") therefore no valid I/O handle can be NULL because it would overlap with that meaning.

I admit this falls short of explicit documentation but it's the closest I can find. You could also cite functions such as OpenProcess that use NULL to indicate errors but obviously they're specific to the type of handle.

In short there's no explicit documentation that covers all types of handles but, as the blog you linked to notes, the very fact that it was a "toss up" whether to use NULL to signal an error is a good indication that it's not a valid handle, And unlike INVALID_HANDLE_VALUE there's no documentation that says "for this specific type of handle NULL is treated as a special, valid, handle value".

(more broadly, it would likely break a lot of things if NULL were ever to become valid but this is supposition on my part).

I/O safety is now [in Rust Nightly]; add a mode to io-lifetimes to use std's types and traits. See the blurb in README.md for more details. [in Rust Nightly]: rust-lang/rust#87329

Implement `AsFd`, `From<OwnedFd>`, and `Into<OwnedFd>` for `UnixListener`. This is a follow-up to rust-lang#87329.

…-io-safety, r=joshtriplett Implement `AsFd` etc. for `UnixListener`. Implement `AsFd`, `From<OwnedFd>`, and `Into<OwnedFd>` for `UnixListener`. This is a follow-up to rust-lang#87329. r? `@joshtriplett`

Fix WinUWP std compilation errors due to I/O safety I/O safety for Windows has landed in rust-lang#87329. However, it does not cover UWP specific parts and prevents all UWP targets from building. See YtFlow/Maple#18. This PR fixes these compile errors when building std for UWP targets.

Nashenas88 · 2021-10-14T18:36:43Z

In case this is useful, we discovered that this change adds ambiguity for code that was relying on where T: From<F>, F: FromRawFd (roughly). This was a simple fix on our side, but we did see this breakage:

[115822/211194] RUST password_authenticator_integration_test exe.unstripped/password_authenticator_integration_test
FAILED: password_authenticator_integration_test exe.unstripped/password_authenticator_integration_test
mkdir -p ./exe.unstripped &&  RUST_BACKTRACE=1 ../../../recipe_cleanup/rustupp6g4sd/bin/rustc --color=always --crate-name password_authenticator_integration_test ../../src/identity/tests/password_a...
error[E0283]: type annotations needed
   --> ../../src/identity/tests/password_authenticator_integration/src/account_manager.rs:128:17
    |
128 |                 fs::File::from(fdio::create_fd(ramdisk_handle).expect("create fd of dev root"));
    |                 ^^^^^^^^^^^^^^ --------------------------------------------------------------- this method call resolves to `T`
    |                 |
    |                 cannot infer type for type parameter `T` declared on the trait `From`
    |
    = note: cannot satisfy `std::fs::File: From<_>`
note: required by `from`

error[E0283]: type annotations needed
   --> ../../src/identity/tests/password_authenticator_integration/src/account_manager.rs:250:9
    |
250 |           fs::File::from(
    |           ^^^^^^^^^^^^^^ cannot infer type for type parameter `T` declared on the trait `From`
251 | /             fdio::create_fd(
252 | |                 dev_root_proxy
253 | |                     .into_channel()
254 | |                     .expect("Could not convert dev root DirectoryProxy into channel")
...   |
257 | |             )
258 | |             .expect("create fd of dev root"),
    | |____________________________________________- this method call resolves to `T`
    |
    = note: cannot satisfy `std::fs::File: From<_>`
note: required by `from`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0283`.

Where fdio::create_fd has a signature of pub fn create_fd<F: FromRawFd>(handle: zx::Handle) -> Result<F, zx::Status>

sunfishcode · 2021-10-16T18:11:54Z

Thanks for reporting this! I've now filed #89955 to track this.

Copy the relevant trait implementations from the Unix default.

Minimally comply with with rust-lang#87329 to avoid breaking tests on L4Re.

adrianheine · 2023-06-13T11:14:52Z

I don't know if there's another way to handle this, but the since of 1.0.0 and 1.1.0 on items are correct for the re-exports under std::os::unix::io but obviously wrong for std::os::fd, which should be since = "1.56.0".

…r=workingjubilee kmc-solid: I/O safety Adds the I/O safety API (rust-lang#87329) for socket file descriptors in [`*-kmc-solid_*`](https://doc.rust-lang.org/nightly/rustc/platform-support/kmc-solid.html) Tier 3 targets. All new public items are gated by the `solid_ext` library feature. This PR adds the following public types and traits: std::os::solid::io::AsFd std::os::solid::io::BorrowedFd std::os::solid::io::OwnedFd std::os::solid::prelude::AsFd (re-export) std::os::solid::prelude::BorrowedFd (re-export) std::os::solid::prelude::OwnedFd (re-export) And trait implementations: From<std::net::TcpListener> for std::os::solid::io::OwnedFd From<std::net::TcpStream> for std::os::solid::io::OwnedFd From<std::net::UdpSocket> for std::os::solid::io::OwnedFd From<std::os::solid::io::OwnedFd> for std::net::TcpListener From<std::os::solid::io::OwnedFd> for std::net::TcpStream From<std::os::solid::io::OwnedFd> for std::net::UdpSocket std::fmt::Debug for std::os::solid::io::BorrowedFd<'_> std::fmt::Debug for std::os::solid::io::OwnedFd std::io::IsTerminal for std::os::solid::io::BorrowedFd<'_> std::io::IsTerminal for std::os::solid::io::OwnedFd std::os::fd::AsRawFd for std::os::solid::io::BorrowedFd<'_> std::os::fd::AsRawFd for std::os::solid::io::OwnedFd std::os::fd::FromRawFd for std::os::solid::io::OwnedFd std::os::fd::IntoRawFd for std::os::solid::io::OwnedFd std::os::solid::io::AsFd for &impl std::os::solid::io::AsFd std::os::solid::io::AsFd for &mut impl std::os::solid::io::AsFd std::os::solid::io::AsFd for Arc<impl std::os::solid::io::AsFd> std::os::solid::io::AsFd for Box<impl std::os::solid::io::AsFd> std::os::solid::io::AsFd for Rc<impl std::os::solid::io::AsFd> std::os::solid::io::AsFd for std::net::TcpListener std::os::solid::io::AsFd for std::net::TcpStream std::os::solid::io::AsFd for std::net::UdpSocket std::os::solid::io::AsFd for std::os::solid::io::BorrowedFd<'_> std::os::solid::io::AsFd for std::os::solid::io::OwnedFd Taking advantage of the above change, this PR also refactors the internal details of `std::sys::solid::net` to match the design of other targets, e.g., by redefining `Socket` as a newtype of `OwnedFd`.

rust-highfive assigned joshtriplett Jul 21, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 21, 2021

sunfishcode mentioned this pull request Jul 21, 2021

Tracking Issue for RFC 3128: I/O Safety #87074

Closed

5 tasks

This comment has been minimized.

Sign in to view

sunfishcode force-pushed the sunfishcode/io-safety branch from fbe1b82 to 712182c Compare July 21, 2021 02:35

This comment has been minimized.

Sign in to view