tokio icon indicating copy to clipboard operation
tokio copied to clipboard

Tokio casts from `&mut [MaybeUninit<u8>]` to `&mut [u8]`

Open erickt opened this issue 3 years ago • 5 comments

Version

1.17.0 Head

Platform

Linux

Description

We're updating our Tokio dependency from 0.2.21 to 1.17.0 in https://fuchsia-review.googlesource.com/c/fuchsia/+/611683, and as part of it we're auditing Tokio. We noticed that tokio::net has a few places that are casting a &mut [MaybeUninit<u8>] to &mut [u8]:

  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/tcp/stream.rs#L336
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/tcp/stream.rs#L722
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/udp.rs#L795
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/udp.rs#L917
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/udp.rs#L982
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/udp.rs#L1197
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/udp.rs#L1373
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/unix/stream.rs#L436
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/unix/datagram/socket.rs#L843
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/unix/datagram/socket.rs#L906
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/unix/datagram/socket.rs#L1045
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/unix/datagram/socket.rs#L1146
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/windows/named_pipe.rs#L600
  • https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/windows/named_pipe.rs#L1338

If I'm reading the unsafe guidelines correctly, this is undefined behavior, or at least considered potentially undefined behavior. I think this was the impetus behind the ReadBuf struct to manage these types.

I think the main source of this is that mio doesn't expose methods that let you work with &mut [MaybeUninit<u8>] types. It'd be great if mio did have methods for these types, so we could push down the actual unsafe blocks into mio, which would make it much easier to review these changes. I'll file a corresponding ticket with mio.

erickt avatar May 13 '22 01:05 erickt

The current status of this is that the only way to avoid it is to bypass the standard library and directly call into libc for the read and write calls.

Darksonn avatar May 13 '22 07:05 Darksonn

Another option that came up in https://github.com/tokio-rs/mio/issues/1574 is that we could change mio to use socket2, instead of std::net, which apparently supports &mut [MaybeUninit<u8>] already.

erickt avatar May 13 '22 17:05 erickt

Well, it's definitely possible that socket2 already has the code for the libc wrappers I mentioned.

Darksonn avatar May 13 '22 18:05 Darksonn

Found another place that's doing the cast: https://github.com/tokio-rs/tokio/blob/master/tokio/src/io/poll_evented.rs#L155

erickt avatar May 13 '22 20:05 erickt

In general, Tokio and many other crates have done it in this way to avoid maintaining a duplicate of the relevant code from the standard library. It involves various platform specific details beyond just making the libc call. That said, if socket2 already has an implementation of that code, I am ok with using it.

Darksonn avatar May 13 '22 21:05 Darksonn