Return the full length of sockaddr_in in socket functions with in/out addrlen
#550
Conversation
Previously the socket funtions did not write the sockaddr_in struct fully and instead always returned the "internal" length, which did not account for the zeroed padding.
|
Can you split the commit into one that updates all the 0x1c to ADDR_STORAGE_LEN and one that does the other stuff? |
| @@ -10,6 +10,7 @@ | |||
| #include <3ds/services/soc.h> | |||
|
|
|||
| #define SYNC_ERROR ENODEV | |||
| #define ADDR_STORAGE_LEN sizeof(struct sockaddr_storage) | |||
There was a problem hiding this comment.
Maybe we should have a _Static_assert(ADDR_STORAGE_LEN == 0x1c);
There was a problem hiding this comment.
My idea was to not hardcode the size to any particular value (this PR is of the intent to revise the size of sockaddr after all, and I remember that the 3DS doesn’t have IPv6 support). It could be a helpful test though, I’ll see whether to add it if I have to make any changes.
There was a problem hiding this comment.
iirc you get an error with any other value
They are already separated 👍 |
Relevant links: rust3ds/ctru-rs#174, discussion on courses of action, original change in libc crate, problematic test.
CC: @ian-h-chamberlain @adryzz @AzureMarker
The kernel implementation of
sockaddrdata, used by allsocservices, is composed of 8 bytes of data. While this is true, thesockaddr_indefinition exposed by libctru is more closely related to the corresponding Linux (and many other systems') definition, having an additional 8 byte padding field (sin_zero). This is a generally favorable design decision for compatibility with most *nix software. However, unlike most Posix-complying implementations, the current implementation inlibctruonly handles the transfer of the 8 bytes actually manipulated by the OS.This particularity imposes 2 "differentiating" factors that may hinder compatiblity with code written for standard *nix platforms:
sockaddr_inmemory are left with uninitialized data (not a big deal since that memory isn't responsibility oflibctru). This also shouldn't be a problem since the implementation of functions with an in/out*addrlenargument correctly specify that only the first 8 bytes were modified (also,sin_zeroisn't supposed to ever be read).sockaddr_instructure (16 bytes), meaning that programs expecting the size given to be the same as the output will be unpleasantly surprised when they find out thesockaddr_instruct is "unexpectedly" oversized. For this reason (and for the fact thatsin_zerois a fundamentally important field for most implementations), platforms such as Linux actually return the full 16 byte length, taking into consideration those 8 trailing bytes. This isn't done bylibctru, which only returns a socket address length of 8.I am fully aware that this is basically a non-existent problem, and as far as I can tell, the Posix standard isn't even broken by having an oversized
sockaddr_indefinition, since data length issues are to be resolved using the*addrlenparameter, and not by "expecting" sizes and lengths to be that of data structures. However, and the case that prompted this change is proof of this, this situation is not really expected to be handled by most programs, which instead believe the platform's socket implementation has to account for the full memory mapping (which is something they usually do).The possible fixes for this problem are:
sin_zerofield from thelibctrudefinition and more thinly wrap the kernel implementation. (might break backward and cross compatibility)sockaddr_infor successful calls to socket functions. Since this is the solution which requires the most changes, I've taken the liberty to include such changes in this PR.This PR doesn't have to be merged or closed as-is. Please take the time to discuss the issue further if this is not what you intend on using.