initialize msg_name with null pointer when msg_name is empty #2530
Conversation
src/sys/socket/mod.rs
Outdated
| let p = mhdr.as_mut_ptr(); | ||
| unsafe { | ||
| (*p).msg_name = address as *mut c_void; | ||
| (*p).msg_name = if S::size() == 0 { ptr::null_mut() } else { (*address).as_mut_ptr() as *mut c_void }; |
There was a problem hiding this comment.
(*address).as_mut_ptr() as *mut c_void
Any idea why it would work after reverting the change made in #2041?
There was a problem hiding this comment.
not sure if things fixed by https://github.com/nix-rust/nix/pull/2041 are still working after that.
Considering this, gentle ping on @JarredAllen, the author of #2041, could you please take a look at this PR?
Context
@pacak is trying to solve #2506, after digging into the previous Nix versions(Thank you for this!), he figured out that the issue is possibly introduced by #2041:
If we can have you, I hope we can solve the following questions:
- The reason behind above change made in Set the length of a socket address when calling
recvmsgon Linux #2041, and possibly why reverting it would fix Re-useMultiHeader<()>in subsequentrecvmmsg()calls #2506. - The things that Set the length of a socket address when calling
recvmsgon Linux #2041 tried to fix would still work after this PR.
There was a problem hiding this comment.
Any idea why it would work after reverting the change made in #2041?
As far as I understand .as_mut_ptr() gives a null pointer in case of (), address as *mut c_void gives a dangling pointer through magic of rustc optimizations.
There was a problem hiding this comment.
I guess just using .as_mut_ptr() would be enough as long as all the instances return the right pointer. Explicit checking for size makes it so it works even if there's a bogus instance.
There was a problem hiding this comment.
As far as I understand .as_mut_ptr() gives a null pointer in case of (), address as *mut c_void gives a dangling pointer through magic of rustc optimizations.
If so
I guess just using .as_mut_ptr() would be enough as long as all the instances return the right pointer.
then yes
not sure if things fixed by #2041 are still working after that.
I think this PR won't break the fix done by #2041 as there is a unit test guarding it.
BTW, let's document why we should use .as_mut_tr() rather than address as *mut c_void because it is so easy to revert the fix if one is not aware of such a subtle different behavior . 😮💨
There was a problem hiding this comment.
Sorry I wasn't around, but thanks for fixing my mistake! I concur with the assessment that this shouldn't break my fix.
The msg_name field points to a caller-allocated buffer that is used to return the source address if the socket is unconnected. The caller should set msg_namelen to the size of this buffer before this call; upon return from a successful call, msg_namelen will contain the length of the returned address. If the application does not need to know the source address, msg_name can be specified as NULL. In case we use () msgname_len gets initialized with 0, but a dangling pointer to the array with msg_name. This works for the first iteration somehow, but after that kernel sets msgname_len to a non-zero and second invocation with the same MultiHeader fails Fixes nix-rust#2506
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
3 participants
The msg_name field points to a caller-allocated buffer that is used to return the source address if the socket is unconnected. The caller should set msg_namelen to the size of this buffer before this call; upon return from a successful call, msg_namelen will contain the length of the returned address. If the application does not need to know the source address, msg_name can be specified as NULL.
In case we use () msgname_len gets initialized with 0, but pointer to the array with msg_name. This works for the first iteration somehow, but after that kernel sets msgname_len to a non-zero and second invocation with the same MultiHeader fails
What does this PR do
Fixes #2506
Checklist:
CONTRIBUTING.md