Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
erickt opened this issue May 13, 2022 · 5 comments
Open

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

erickt opened this issue May 13, 2022 · 5 comments
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-net Module: tokio/net

Comments

@erickt
Copy link
Contributor

erickt commented May 13, 2022

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]:

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 erickt added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels May 13, 2022
@Darksonn Darksonn added the M-net Module: tokio/net label May 13, 2022
@Darksonn
Copy link
Contributor

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.

@erickt
Copy link
Contributor Author

erickt commented May 13, 2022

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

@Darksonn
Copy link
Contributor

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

@erickt
Copy link
Contributor Author

erickt commented May 13, 2022

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

@Darksonn Darksonn changed the title UB, or at least risky code, in tokio::net casting &mut [MaybeUninit<u8>] to &mut [u8] Tokio casts from &mut [MaybeUninit<u8>] to &mut [u8] May 13, 2022
@Darksonn
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-net Module: tokio/net
Projects
None yet
Development

No branches or pull requests

2 participants