OpenBSD fix long socket addresses #118349
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
O-unix
This comment has been minimized.
This comment has been minimized.
notgull
force-pushed
the
openbsd-addr
branch
from
d6d32c3
to
781e3c8
Compare
This comment has been minimized.
This comment has been minimized.
|
@notgull could you please adjust the comment saying it is an OpenBSD bug, as it is the intented behaviour for OpenBSD ? |
There is an OpenBSD bug where the "len" returned by functions like "getsockname" is too long and makes it so the resulting address contains zero bytes. While this isn't a problem for C code, where strings are null terminated anyways, it's a problem for Rust. This commit fixes this issue by adding a check that truncates the address length to the first zero when OpenBSD is detected. If there are no zeroes, it just uses the original length provided by the system. Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: John Nunley <dev@notgull.net>
This comment has been minimized.
This comment has been minimized.
|
r? libs |
library/std/src/os/unix/net/addr.rs
Outdated
| .map_or(len, |new_len| (new_len + sun_path_offset(&addr)) as libc::socklen_t); | ||
| } | ||
|
|
||
| if addr.sun_family != libc::AF_UNIX as libc::sa_family_t { |
There was a problem hiding this comment.
It looks like this is now run even when len == 0. That might be the right behavior, but it seems like a change in behavior that should go to a separate PR.
There was a problem hiding this comment.
I've added back the original behavior.
There was a problem hiding this comment.
I don't think the current PR actually adds back the original behavior, but thinking more on this it's not clear to me what the original intent behind placing the family check in an else was. I'm not able to find when this code was originally introduced quickly (maybe in #51553 but that seems surprising to me...), but I think my preference would be to check the family before the length (i.e., not guard it with any length comparisons). That seems like the most intuitive behavior for this function and I'd expect that it should work, if we see tests fail etc. we can revert and add some comments describing why.
(The current PR is adding a len == 0 check before returning an error is looking at the updated length after it may have been overwritten, though only in the OpenBSD case could it get overwritten to a zero I think).
rustbot
added
S-waiting-on-author
Signed-off-by: John Nunley <dev@notgull.net>
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
Signed-off-by: John Nunley <dev@notgull.net>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: John Nunley <dev@notgull.net>
|
Please also keep commits squashed. |
Mark-Simulacrum
added
S-waiting-on-author
|
Closing as I do not have the time to rebase this |
Original diff from @notgull in rust-lang#118349, small changes from me. on OpenBSD, getsockname(2) returns the actual size of the socket address, and not the len of the content. Figure out the length for ourselves. see https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 Fixes rust-lang#116523
|
@notgull thanks for your work. |
Original diff from @notgull in rust-lang#118349, small changes from me. on OpenBSD, getsockname(2) returns the actual size of the socket address, and not the len of the content. Figure out the length for ourselves. see https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 Fixes rust-lang#116523
…Mark-Simulacrum OpenBSD fix long socket addresses Original diff from `@notgull` in rust-lang#118349, small changes from me. on OpenBSD, getsockname(2) returns the actual size of the socket address, and not the len of the content. Figure out the length for ourselves. see https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 Fixes rust-lang#116523
…Mark-Simulacrum OpenBSD fix long socket addresses Original diff from ``@notgull`` in rust-lang#118349, small changes from me. on OpenBSD, getsockname(2) returns the actual size of the socket address, and not the len of the content. Figure out the length for ourselves. see https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 Fixes rust-lang#116523
Rollup merge of rust-lang#123779 - semarie:notgull-openbsd-socket, r=Mark-Simulacrum OpenBSD fix long socket addresses Original diff from ``@notgull`` in rust-lang#118349, small changes from me. on OpenBSD, getsockname(2) returns the actual size of the socket address, and not the len of the content. Figure out the length for ourselves. see https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 Fixes rust-lang#116523
Original diff from @notgull in rust-lang#118349, small changes from me. on OpenBSD, getsockname(2) returns the actual size of the socket address, and not the len of the content. Figure out the length for ourselves. see https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 Fixes rust-lang#116523
There is an OpenBSD bug where the "len" returned by functions like "getsockname" is too long and makes it so the resulting address contains zero bytes. While this isn't a problem for C code, where strings are null terminated anyways, it's a problem for Rust.
This commit fixes this issue by adding a check that truncates the address length to the first zero when OpenBSD is detected. If there are no zeroes, it just uses the original length provided by the system.
Closes #116523