summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlejandro Colomar <alx@kernel.org>2023-01-20 00:38:11 +0100
committerAlejandro Colomar <alx@kernel.org>2023-08-29 16:05:21 +0200
commit5ee1d895cc697f815e0b634b9c9cca7b0011dd5b (patch)
treeec885d6893dbf04baab60d12a8c572936f326d94
parentdfe8c445883a50a55564b02b6957257bfc510db4 (diff)
socket: Implement sockaddr_storage with an anonymous unionsockaddr
The historical design of `sockaddr_storage` makes it impossible to use without breaking strict aliasing rules. Not only this type is unusable, but even the use of other `sockaddr_*` structures directly (when the programmer only cares about a single address family) is also incorrect, since at some point the structure will be accessed as a `sockaddr`, and that breaks strict aliasing rules too. So, the only way for a programmer to not invoke Undefined Behavior is to declare a union that includes `sockaddr` and any `sockaddr_*` structures that are of interest, which allows later accessing as either the correct structure or plain `sockaddr` for the sa_family. This patch fixes sockaddr_storage to remove UB on its uses and make it that structure that everybody should be using. It also allows removing many casts in code that needs to pass a sockaddr as a side effect. The following is an example of how this improves both existing code and new code: void foo(foo) { struct old_sockaddr_storage oss; struct new_sockaddr_storage nss; // ... (initialize oss and nss) inet_sockaddr2str(&nss.sa); // correct (and has no casts) inet_sockaddr2str((struct sockaddr *)&oss); // UB inet_sockaddr2str((struct sockaddr *)&nss); // correct } /* This function is correct, as far as the accessed object has the * type we're using. That's only possible through a `union`, since * we're accessing it with 2 different types: `sockaddr` for the * `sa_family` and then the appropriate subtype for the address * itself. */ const char * inet_sockaddr2str(const struct sockaddr *sa) { struct sockaddr_in *sin; struct sockaddr_in6 *sin6; static char buf[MAXHOSTNAMELEN]; switch (sa->sa_family) { case AF_INET: sin = (struct sockaddr_in *) sa; inet_ntop(AF_INET, &sin->sin_addr, buf, NITEMS(buf)); return buf; case AF_INET6: sin6 = (struct sockaddr_in6 *) sa; inet_ntop(AF_INET6, &sin6->sin6_addr, buf, NITEMS(buf)); return buf; default: errno = EAFNOSUPPORT; return NULL; } } While it's not necessary to do the same for `sockaddr`, it might still be interesting to so, since it will allow removing many casts in the implementation of many libc functions. Link: <https://lore.kernel.org/linux-man/2860541.uBSZ6KuyZf@portable-bastien/T/> Link: <https://inbox.sourceware.org/libc-alpha/0f25d60f-f183-b518-b6c1-6d46aa63ee57@gmail.com/T/> Link: <https://stackoverflow.com/a/42190913/6872717> Link: <https://software.codidact.com/posts/287748> Cc: Bastien Roucariès <rouca@debian.org> Cc: Eric Blake <eblake@redhat.com> Cc: Zack Weinberg <zack@owlfolio.org> Cc: Stefan Puiu <stefan.puiu@gmail.com> Cc: Igor Sysoev <igor@sysoev.ru> Signed-off-by: Alejandro Colomar <alx@kernel.org>
-rw-r--r--bits/socket.h11
-rw-r--r--sysdeps/mach/hurd/bits/socket.h11
-rw-r--r--sysdeps/unix/sysv/linux/bits/socket.h11
3 files changed, 24 insertions, 9 deletions
diff --git a/bits/socket.h b/bits/socket.h
index a22ff3b07b..13ba3ae358 100644
--- a/bits/socket.h
+++ b/bits/socket.h
@@ -168,9 +168,14 @@ struct sockaddr
struct sockaddr_storage
{
- __SOCKADDR_COMMON (ss_); /* Address family, etc. */
- char __ss_padding[_SS_PADSIZE];
- __ss_aligntype __ss_align; /* Force desired alignment. */
+ union
+ {
+ __SOCKADDR_COMMON (ss_); /* Address family, etc. */
+ struct sockaddr sa;
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr_un sun;
+ };
};
diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h
index c2392bed80..e882003e43 100644
--- a/sysdeps/mach/hurd/bits/socket.h
+++ b/sysdeps/mach/hurd/bits/socket.h
@@ -172,9 +172,14 @@ struct sockaddr
struct sockaddr_storage
{
- __SOCKADDR_COMMON (ss_); /* Address family, etc. */
- char __ss_padding[_SS_PADSIZE];
- __ss_aligntype __ss_align; /* Force desired alignment. */
+ union
+ {
+ __SOCKADDR_COMMON (ss_); /* Address family, etc. */
+ struct sockaddr sa;
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr_un sun;
+ };
};
diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
index 57b05715be..012f1007bd 100644
--- a/sysdeps/unix/sysv/linux/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/bits/socket.h
@@ -195,9 +195,14 @@ struct sockaddr
struct sockaddr_storage
{
- __SOCKADDR_COMMON (ss_); /* Address family, etc. */
- char __ss_padding[_SS_PADSIZE];
- __ss_aligntype __ss_align; /* Force desired alignment. */
+ union
+ {
+ __SOCKADDR_COMMON (ss_); /* Address family, etc. */
+ struct sockaddr sa;
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr_un sun;
+ };
};