sockaddr_storage shouldn't have internal padding bytes

[stevenengler]

A common use-case of sockaddr_storage is to store a socket address in a sockaddr_storage, inspect the ss_family field and the socket address length, and then cast it to the correct socket address type (for example sockaddr_in). For example, this is what the nix crate currently does using a union: https://github.com/nix-rust/nix/blob/fbebb21dd8df447a1408795b4b5706d9ca6c55df/src/sys/socket/addr.rs#L1537-L1540

The rust libc library defines sockaddr_storage as the following on Linux:

pub struct sockaddr_storage {
    pub ss_family: sa_family_t,
    __ss_align: ::size_t,
    #[cfg(target_pointer_width = "32")]
    __ss_pad2: [u8; 128 - 2 * 4],
    #[cfg(target_pointer_width = "64")]
    __ss_pad2: [u8; 128 - 2 * 8],
}

On Linux x86-64, this results in the following representation:

type: `unix::linux_like::sockaddr_storage`: 128 bytes, alignment: 8 bytes
    field `.ss_family`: 2 bytes
    padding: 6 bytes
    field `.__ss_align`: 8 bytes, alignment: 8 bytes
    field `.__ss_pad2`: 112 bytes

From what I understand, the 6 padding bytes in sockaddr_storage should cause a cast from memory initialized as a sockaddr_storage to another type like sockaddr_in to invoke UB since these padding bytes are considered to be uninitialized memory. For example these padding bytes of sockaddr_storage overlap with the sockaddr_in.sin_port field, so reading the sin_port field after casting from a sockaddr_storage would be UB. And in the nix case, the ss: libc::sockaddr_storage = mem::zeroed() may also not zero these 6 padding bytes, possibly leading to UB when it's later read as a different socket address type if those padding bytes weren't overwritten during the ptr::copy.

On recent glibc versions (such as 2.31 on Ubuntu 20.04), they define sockaddr_storage as:

#define __ss_aligntype  unsigned long int
#define _SS_PADSIZE \
  (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))

struct sockaddr_storage
  {
    __SOCKADDR_COMMON (ss_);    /* Address family, etc.  */
    char __ss_padding[_SS_PADSIZE];
    __ss_aligntype __ss_align;  /* Force desired alignment.  */
  };

Since they have the padding/alignment fields in the opposite order from rust's libc definition (have the char array field first), there are no padding bytes:

printf("sockaddr_storage.ss_family: (offset) %ld, (size) %ld\n",
       (void *)&addr.ss_family - (void *)&addr,
       sizeof(addr.ss_family));
printf("sockaddr_storage.__ss_padding: (offset) %ld, (size) %ld\n",
       (void *)&addr.__ss_padding - (void *)&addr,
       sizeof(addr.__ss_padding));
printf("sockaddr_storage.__ss_align: (offset) %ld, (size) %ld\n",
       (void *)&addr.__ss_align - (void *)&addr,
       sizeof(addr.__ss_align));
printf("sockaddr_storage: (size) %ld\n", sizeof(addr));

sockaddr_storage.ss_family:	(offset) 0,	(size) 2
sockaddr_storage.__ss_padding:	(offset) 2,	(size) 118
sockaddr_storage.__ss_align:	(offset) 120,	(size) 8
sockaddr_storage: (size) 128

The order of these fields was changed in this 2016 glibc patch: "Make padding in struct sockaddr_storage explicit [BZ #20111]" https://patchwork.sourceware.org/project/glibc/patch/fea4fa60-be9c-7cc2-b09c-c48845fe603f@redhat.com/

[asomers]

Since the __ss_align and __ss_pad2 fields are private, it should be no problem to change them to make all padding bytes explicit. Would you submit a PR for that?

[stevenengler]

Since the __ss_align and __ss_pad2 fields are private, it should be no problem to change them to make all padding bytes explicit. Would you submit a PR for that?

I made #3010 to change the structure on Linux. I'm not very familiar with other platforms, but I think this should be changed on Fuchsia as well?

libc/src/fuchsia/mod.rs

Lines 910 to 914 in 73c25f4

	pub struct sockaddr_storage {
	pub ss_family: sa_family_t,
	__ss_align: ::size_t,
	__ss_pad2: [u8; 128 - 2 * 8],
	}

[stevenengler]

Closing since #3010 fixes this on Linux and Fuchsia. If anyone finds other platforms with this issue it can be reopened, but I'm not familiar enough with other platforms to know if it is or not.