nix icon indicating copy to clipboard operation
nix copied to clipboard

Making sure that sa_family is always initialized

Open Jan561 opened this issue 1 year ago • 3 comments
trafficstars

Alternative to #2235

Checklist:

  • [x] I have read CONTRIBUTING.md
  • [x] I have written necessary tests and rustdoc comments
  • [x] A change log has been added if this PR modifies nix's API

Jan561 avatar Dec 01 '23 00:12 Jan561

Actually this fixes nothing as recvmsg doesn't even call SockaddrLike::from_raw. Instead, it just assumes correct initialization and doesn't check anything

https://github.com/nix-rust/nix/blob/83dfd90741756041977ad91ae123d2ab1b27c13f/src/sys/socket/mod.rs#L2042

Jan561 avatar Dec 01 '23 00:12 Jan561

The change to SockaddrLikePriv::as_mut_ptr is unnecessary. You can do this manipulation in pack_mhdr_to_receive on raw pointers directly.

Actually that's not possible, the address: *mut S argument is not NULL for S = (), it is just dangling.

That's how this function is called, with address being initialized with MaybeUninit::<S>::uninit():

https://github.com/nix-rust/nix/blob/a4e0c9544cbc7ec092fbda8a7c69afa7b65438b2/src/sys/socket/mod.rs#L2035

Also, could you please add a test that reproduces the original bug?

Adding a test for this isn't that trivial, since currently the address field is "random" if not initialized by the syscall.

But the set of possible family values is quite small on BSDs (sa_family_t = u8), so I think with a test that calls recvmsg 500 or 1000 times on a connected socket we can be almost certain that one iteration will have the family field preset to e.g. INET or INET6.

But before we can add this test, we need to fix recvmsg

Jan561 avatar Dec 02 '23 17:12 Jan561

I'd also consider the recvmsg situation a blocking issue for this PR

Jan561 avatar Dec 02 '23 17:12 Jan561