Why is FromRawFd unsafe?

Hello

I'm looking at the FromRawFd trait. The method there is unsafe, with a vague explanation that violation of contract to be the sole owner of the file descriptor might trigger memory unsafety:

This function is also unsafe as the primitives currently returned have the contract that they are the sole owner of the file descriptor they are wrapping. Usage of this function could accidentally allow violating this contract which can cause memory unsafety in code that relies on it being true.

However, I can't think of an example where this would happen ‒ if I violate this for eg. a File, then the same fd can get closed multiple times, due to recycling it after being closed it could close something really unrelated, which is bad, but would still result in EBADFD returned all over the place, not UB.

The only thing I could find was this really old issue: FromRawFd should be unsafe · Issue #1043 · rust-lang/rfcs · GitHub. Nevertheless, if I could cause the UB by writing invalid data to the file, I can get to that by opening the same file again, or by using try_clone on the file.

Does anyone know of an actual problem this might trigger? Should the unsafety be described better? If there's an actual problem, is the write in nix unsound?

The reason why I ask is, I have a function in my library that takes ownership of a raw file descriptor, writes there and eventually closes it. In that regard, it is really similar to this trait and I wonder if it should be unsafe for the same reasons ‒ but I'm not sure if any actual ones exist, or if it is unsafe simply to allow a hypothetical type to rely on the fact it is the sole owner for its own internal data structures.

Thank you for advice and historical links for further my archeological research.

3 Likes

Yes, this seems to date from the time safety was a looser term (remember that at some point mem::forget was unsafe), and where it was thought that there could exist a non-unsafe API over memory-mapped files.
I guess right now it is more of a vestigial lint more than anything else, given how Rust will anyways not have control over filesystems.

That being said, while not causing UB, having your file descriptor being closed under your nose can indeed lead to recycling a file descriptor from somewhere else, and while not UB memory-wise, it can lead to pretty severe bugs. For instance, could you imagine the following code not printing the contents of the Cargo.toml file?

fn foo (read_metadata: impl FnOnce(&'_ fs::File) -> io::Result<()>)
  -> io::Result<()>
{
    let mut cargo_toml = fs::File::open("Cargo.toml")?;
    read_metadata(&cargo_toml)?;
    println!("`Cargo.toml` file contents:\n{}", {
        let mut s = String::new();
        cargo_toml.read_to_string(&mut s)?;
        s
    });
    Ok(())
}

To be honest, this is the same category as CStr::from_bytes_with_nul_unchecked saying it is UB to feed it b"Hell\0, W\0rld!\0", even though it will "just" truncate the string in that case.

6 Likes

How about an example. Here are two playgrounds.
Good fd ex
runtime error ex
Files created using from_raw_fd assume ownership of the file descriptor. But you could give a fd that doesn't actually exist. Rust will only know when you try to modify the file, which will either work just fine or give runtime errors.

Some literature about file descriptors for your reading pleasure.

@Yandros, when you say Rust will not have control over filesystems, what do you mean? That's unclear to me: It sounds like you mean that Rust doesn't run like some other language does?

Thank you for the input. Yes, I understand this could lead to very severe bugs, but as I've seen it said somewhere, Rust doesn't prevent you from deleting your production database, it just makes sure not to segfault doing that.

As for the from_bytes_with_nul_unchecked, I believe it is unsafe because you can pass something without any nul bytes and the program would run off the end of the slice once used.

So it seems my function can still stay as safe API ‒ as I said closing it for someone else is indeed bad and one will get EBADFD over the place, but not UB by definition. But maybe I should add some warning to it. It is part of low-level API, so folks are likely to be careful around it anyway.

@Yandros that is indeed cool example! (I still think this doesn't deserve unsafe, though, provided things like AssertUnwindSafe are not marked unsafe)

1 Like

I think @Yandros is referring to

The provided slice must be nul-terminated and not contain any interior nul bytes.

emphasis mine. The note about interior nul bytes seems unnecessary solely from a safety standpoint.

2 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.