Re: [PATCH] socket_local46: Make room on the stack for IPv6 sockaddrs

[Jesse Young]

On Sun, 26 Jul 2015 21:29:31 +0200
Laurent Bercot ska-skaw...@skarnet.org wrote:

 On 26/07/2015 21:15, Jesse Young wrote:
  byte_copy() reads past the end of the sockaddr structure because it
  isn't sufficiently large enough to handle sockaddr_in6 addresses
  resulting in undefined behavior. armv6-alpine-linux-muslgnueabihf-gcc
  5.1.0-r0 generates the UDF instruction in this case, causing programs
  to SIGILL.
 
   Ah, good catch. sockaddr used to work, but it's stricto sensu incorrect -
 and musl is very touchy with standards.
   I believe the right fix is to use sockaddr_storage, though. I'll commit
 a fix tomorrow.
   Thanks !
 

I don't think sockaddr_storage is strictly necessary in this instance.
getsockname() will truncate its result, but socket_local46() won't
use that result unless sa_family is AF_INET or AF_INET6. That being
said, the only penalty to using sockaddr_storage is the extra space, and
it'd be more robust in cases of buggy implementations of getsockname().

Jesse

[PATCH] socket_local46: Make room on the stack for IPv6 sockaddrs

[Jesse Young]

byte_copy() reads past the end of the sockaddr structure because it
isn't sufficiently large enough to handle sockaddr_in6 addresses
resulting in undefined behavior. armv6-alpine-linux-muslgnueabihf-gcc
5.1.0-r0 generates the UDF instruction in this case, causing programs
to SIGILL.
---
 src/libstddjb/socket_local46.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/libstddjb/socket_local46.c b/src/libstddjb/socket_local46.c
index 448160c..611eb37 100644
--- a/src/libstddjb/socket_local46.c
+++ b/src/libstddjb/socket_local46.c
@@ -12,21 +12,23 @@
 
 int socket_local46 (int s, ip46_t *ip, uint16 *port)
 {
-  struct sockaddr sa ;
+  union {
+struct sockaddr_in6 sa6 ;
+struct sockaddr_in sa4 ;
+struct sockaddr sa ;
+  } sa ;
   socklen_t dummy = sizeof sa ;
-  if (getsockname(s, sa, dummy)  0) return -1 ;
-  if (sa.sa_family == AF_INET6)
+  if (getsockname(s, sa.sa, dummy)  0) return -1 ;
+  if (sa.sa.sa_family == AF_INET6)
   {
-register struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)sa ;
-byte_copy(ip-ip, 16, (char const *)sa6-sin6_addr.s6_addr) ;
-uint16_unpack_big((char *)sa6-sin6_port, port) ;
+byte_copy(ip-ip, 16, (char const *)sa.sa6.sin6_addr.s6_addr) ;
+uint16_unpack_big((char *)sa.sa6.sin6_port, port) ;
 ip-is6 = 1 ;
   }
-  else if (sa.sa_family == AF_INET)
+  else if (sa.sa.sa_family == AF_INET)
   {
-register struct sockaddr_in *sa4 = (struct sockaddr_in *)sa ;
-byte_copy(ip-ip, 4, (char const *)sa4-sin_addr.s_addr) ;
-uint16_unpack_big((char *)sa4-sin_port, port) ;
+byte_copy(ip-ip, 4, (char const *)sa.sa4.sin_addr.s_addr) ;
+uint16_unpack_big((char *)sa.sa4.sin_port, port) ;
 ip-is6 = 0 ;
   }
   else return (errno = EAFNOSUPPORT, -1) ;
-- 
2.4.6