Skip to content

Panic on Unix-domain sockets with length 108 #1316

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

Closed
sunfishcode opened this issue Feb 8, 2025 · 2 comments · Fixed by #1371
Closed

Panic on Unix-domain sockets with length 108 #1316

sunfishcode opened this issue Feb 8, 2025 · 2 comments · Fixed by #1371

Comments

@sunfishcode
Copy link
Member

On Linux, Unix-domain sockets with a path of length 108 don't get a terminator within sun_path:

https://man7.org/linux/man-pages/man7/unix.7.html#BUGS

Rustix currently panics on such paths:

#1004 (comment)

@kevinmehall
Copy link
Contributor

The best way to fix this probably involves allowing SocketAddrUnix to hold the 108 byte path. But, since SocketAddrUnix wraps sockaddr_un, it does not have space for 108 bytes plus a nul terminator, and that's a problem for SocketAddrUnix::path returning CStr.

It seems like Path or OsStr would be a more appropriate return type for that function anyway -- may want that breaking change for 1.0.

sunfishcode added a commit that referenced this issue Mar 3, 2025
Unix-domain socket address can be long enough that the NUL terminator does
not fit. Handle this case by making `path()` return a `Cow<CStr>` and
adding a NUL terminator as needed.

Also add a `path_bytes()` function for returning the raw bytes.

Fixes #1316.
@sunfishcode
Copy link
Member Author

The best way to fix this probably involves allowing SocketAddrUnix to hold the 108 byte path. But, since SocketAddrUnix wraps sockaddr_un, it does not have space for 108 bytes plus a nul terminator, and that's a problem for SocketAddrUnix::path returning CStr.

It seems like Path or OsStr would be a more appropriate return type for that function anyway -- may want that breaking change for 1.0.

Thanks for pointing out that this should be fixed for 1.0. I've now filed #1371 as a fix. A complication with OsStr is that this code gets used in no-std contexts. I think we can instead have path return a Cow<CStr> so that it can return a CString when it needs to allocate to add a NUL, and add new path_bytes function which returns the raw bytes with the NUL excluded.

sunfishcode added a commit that referenced this issue Mar 3, 2025
* Handle non-NUL-terminated strings in `SocketAddrUnix`.

Unix-domain socket address can be long enough that the NUL terminator does
not fit. Handle this case by making `path()` return a `Cow<CStr>` and
adding a NUL terminator as needed.

Also add a `path_bytes()` function for returning the raw bytes.

Fixes #1316.

* Allocated `SocketAddrUnix::path()` with correct length (#1372)

Using `.to_owned()` + `.push()` will cause a reallocation, because the
initially allocated array with be one byte too short.

We can use `CString::from_vec_with_nul_unchecked()` because it is a
known invariant that the input does not contain any `NUL`s, not even the
terminating `NUL`.

---------

Co-authored-by: René Kijewski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants