libctru icon indicating copy to clipboard operation
libctru copied to clipboard

Return the full length of `sockaddr_in` in socket functions with in/out addrlen

Open Meziu opened this issue 1 year ago • 2 comments

Relevant links: https://github.com/rust3ds/ctru-rs/issues/174, discussion on courses of action, original change in libc crate, problematic test. CC: @ian-h-chamberlain @adryzz @AzureMarker

The kernel implementation of sockaddr data, used by all soc services, is composed of 8 bytes of data. While this is true, the sockaddr_in definition 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 in libctru only 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:

  1. The padding field is never (internally) overwritten to be actually zeroed. In most situations that means that programs passing in a pointer to sockaddr_in memory are left with uninitialized data (not a big deal since that memory isn't responsibility of libctru). This also shouldn't be a problem since the implementation of functions with an in/out *addrlen argument correctly specify that only the first 8 bytes were modified (also, sin_zero isn't supposed to ever be read).
  2. The returned length, as specified above being always 8 bytes, is less than the size of the sockaddr_in structure (16 bytes), meaning that programs expecting the size given to be the same as the output will be unpleasantly surprised when they find out the sockaddr_in struct is "unexpectedly" oversized. For this reason (and for the fact that sin_zero is 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 by libctru, 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_in definition, since data length issues are to be resolved using the *addrlen parameter, 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:

  • Do nothing about it. (not cool)
  • Remove the sin_zero field from the libctru definition and more thinly wrap the kernel implementation. (might break backward and cross compatibility)
  • Return the full length of sockaddr_in for 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.

Meziu avatar Sep 09 '24 16:09 Meziu

Can you split the commit into one that updates all the 0x1c to ADDR_STORAGE_LEN and one that does the other stuff?

mtheall avatar Sep 09 '24 16:09 mtheall

Can you split the commit into one that updates all the 0x1c to ADDR_STORAGE_LEN and one that does the other stuff?

They are already separated 👍

Meziu avatar Sep 09 '24 18:09 Meziu

It would be great if this could get some movement!

sardap avatar Jan 17 '25 20:01 sardap