Fix using SockaddrStorage to store Unix domain addresses on Linux #1871
Conversation
Since it has variable length, the user of a sockaddr_un must keep track of its true length. On the BSDs, this is handled by the builtin sun_len field. But on Linux-like operating systems it isn't. Fix this bug by explicitly tracking it for SockaddrStorage just like we already do for UnixAddr. Fixes nix-rust#1866
asomers
force-pushed
the
sockaddr_storage_unix_addr_linux
branch
from
63403e7
to
fa62534
Compare
|
@stevenengler could you test your code with this PR please? |
| if i32::from(ss.ss_family) == libc::AF_UNIX { | ||
| // Safe because we UnixAddr is strictly smaller than | ||
| // SockaddrStorage, and we just initialized the structure. | ||
| (*(&mut ss as *mut libc::sockaddr_storage as *mut UnixAddr)).sun_len = len as u8; | ||
| } |
There was a problem hiding this comment.
A libc::sockaddr_storage was initialized above, but I don't think that means that those bytes represent a valid UnixAddr? For example libc::sockaddr_storage has 6 padding bytes on Linux x86-64 that may not be zeroed, and reading those uninitialized bytes as a UnixAddr would be UB. I think this would only occur if len is less than 8 (sizeof(u16) + 6). Edit: And only if the UnixAddr were to read bytes past len. But as long as internally UnixAddr makes sure to never read past len - offsetof(sun_path) bytes of sun_path, then I think it would be fine.
There was a problem hiding this comment.
The docs for mem::zeroed say that padding bytes may not be initialized. But if you look at the source, they are, at least in the current implementation. So I don't think there's a problem to merge this PR. But we should still fix the issue you raised with libc.
Yep, my example code works with this PR. Thanks for looking into this! |
|
bors r=rtzoeller |
Use it in the from_sockaddr_un_abstract_unnamed test. That test and this method were introduced by PRs nix-rust#1871 and nix-rust#1857, which crossed each other.
Since it has variable length, the user of a sockaddr_un must keep track of its true length. On the BSDs, this is handled by the builtin sun_len field. But on Linux-like operating systems it isn't. Fix this bug by explicitly tracking it for SockaddrStorage just like we already do for UnixAddr.
Fixes #1866