quinn: Invalid assumption about SocketAddrV{4,6} layout
It looks like we’re assuming that std::net::Ipv4 matches the libc sockaddr_in (same for IPv6), for example in https://github.com/quinn-rs/quinn/blob/main/quinn/src/platform/unix.rs#L357. However, the standard library never guaranteed this, and in fact is looking at changing this in https://github.com/rust-lang/rust/pull/78802. We should fix our usage.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 32 (13 by maintainers)
Commits related to this issue
- Update quinn to 0.6.2 This fixes an issue about quinn assuming a specific layout for socket addresses: https://github.com/quinn-rs/quinn/issues/968 This might cause bugs on newer compilers. — committed to est31/mimas by est31 3 years ago
- Auto merge of #78802 - faern:simplify-socketaddr, r=joshtriplett Implement network primitives with ideal Rust layout, not C system layout This PR is the result of this internals forum thread: https:... — committed to rust-lang-ci/rust by bors 2 years ago
- Auto merge of #78802 - faern:simplify-socketaddr, r=joshtriplett Implement network primitives with ideal Rust layout, not C system layout This PR is the result of this internals forum thread: https:... — committed to tcdi/postgrestd by bors 2 years ago
Thanks to @est31, I’ve just published 0.5.4 and 0.6.2 releases that contain the relevant fixes.
And yes, I’m willing to help.
Advisory PR submitted: https://github.com/RustSec/advisory-db/pull/804.
FWIW, I also brought up the idea of yanking our buggy versions on Matrix, so we’ll likely do that too (though we may take a few days before yanking 0.6.1). Do you think we should also put out a RustSec advisory for quinn, as you’ve done for the others?
Done.
Can we get a 0.6.x branch? I can then prepare PRs for 0.5.x and 0.6.x.
According to crates.io downloads, everything but 0.6.1 is practically unused. In the chart, the next most popular point is Other, which contains everything pre-0.4.0.
It’s definitely my plan to get this backported to older quinn versions. How far back, no idea.
I’d always prefer correctness over speed. If I were you I would fix the
SocketAddrcasts first and optimize them later (if benchmarks shows they are a bottleneck). I still think you overestimate how much time the conversion take.I agree with what appears to be your assessment (that we should try to avoid the conversion on every send/recv).
I’m guessing they would still compile, just get issues at run-time.
Pretty sure the relevant code hasn’t changed all that much between releases.