Avoid breaking strict aliasing in IP_AS_V{4,6}

GCC complains (one example, more in tcp.h):

In file included from src/main.c:41:
src/tcp.h: In function ‘ip_check_equal_v4’:
src/tcp.h:29:31: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   29 | #define IP_AS_V4(storage, f) ((struct sockaddr_in *)&(storage))->sin_##f
      |                              ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/tcp.h:67:14: note: in expansion of macro ‘IP_AS_V4’
   67 |     { return IP_AS_V4(a, addr).s_addr == IP_AS_V4(b, addr).s_addr; }
      |              ^~~~~~~~

storage (a) here is a pointer to sockaddr_storage which is either backed
by a sockaddr_in or sockaddr_in6 struct (here, it would be good if it is
the former, we decided based on sa_family in ip_check_equal). Referencing
it means we have a pointer to a pointer to sockaddr_storage here, which
we then cast to a pointer to sockaddr_in. Our so type-punned pointer is
then dereferenced breaking strict-aliasing as this pointer as well as
our original pointer share a memory location and both could be used to
change it even through they are of different types.

We can avoid this situation simply by removing the reference as then it
is just casting a pointer to a different type (which in this case is
legal as storage is really a sockaddr_in).

Removing the reference breaks users of the macro who do not feed it a
pointer to a sockaddr_storage, so while the warnings were all produced
by tcp.h, we end up changing code everywhere else to resolve them –
usually by just taking a reference or not dereferencing there.

As this is the only instance of (detected) strict-aliasing breakage
the Makefile is also changed to no longer build the entirety of
tvheadend with strict-aliasing rules disabled. Similar for the disabled
stringop warnings which weren't (correctly) triggered in the current
code.

As a small bonus, this also prevents gcc-11 from tripping with a false
positive over the previous tcp change in c0f616e / #1473.

diff --git a/Makefile b/Makefile
index 1f1c2e36d729e946091883432f795e284047170a..39748e32d11b3a6d3111c9c628e6b5f8cf570fdf 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -51,10 +51,7 @@ CFLAGS  += -Werror
 endif
 CFLAGS  += -Wall -Wwrite-strings -Wno-deprecated-declarations
 CFLAGS  += -Wmissing-prototypes
-CFLAGS  += -fms-extensions -funsigned-char -fno-strict-aliasing
-ifeq ($(COMPILER), gcc)
-CFLAGS  += -Wno-stringop-truncation -Wno-stringop-overflow
-endif
+CFLAGS  += -fms-extensions -funsigned-char
 CFLAGS  += -D_FILE_OFFSET_BITS=64
 CFLAGS  += -I${BUILDDIR} -I${ROOTDIR}/src -I${ROOTDIR}
 ifeq ($(CONFIG_ANDROID),yes)

diff --git a/src/tcp.c b/src/tcp.c
index 6a21bab962149e40af915454bd85459ac4a72815..8ff5587301cfe57cd2815e3ba16df75def8f12de 100644 (file)
--- a/src/tcp.c
+++ b/src/tcp.c
@@ -94,7 +94,7 @@ ip_check_is_local_address
     if (!ifaddr || !ifnetmask) continue;
     if (ifaddr->ss_family != local->ss_family) continue;
     if (!any_address && !ip_check_equal(ifaddr, local)) continue;
-    ret = !!ip_check_in_network_v4(ifaddr, ifnetmask, peer);
+    ret = !!ip_check_in_network(ifaddr, ifnetmask, peer);
     if (ret) {
       if (used_local)
         memcpy(used_local, ifaddr, sizeof(struct sockaddr));
@@ -1051,9 +1051,9 @@ tcp_default_ip_addr ( struct sockaddr_storage *deflt, int family )
   }
 
   if (ss.ss_family == AF_INET)
-    IP_AS_V4(ss, port) = 0;
+    IP_AS_V4(&ss, port) = 0;
   else
-    IP_AS_V6(ss, port) = 0;
+    IP_AS_V6(&ss, port) = 0;
 
   memset(deflt, 0, sizeof(*deflt));
   memcpy(deflt, &ss, ss_len);
@@ -1092,9 +1092,9 @@ tcp_server_bound ( void *server, struct sockaddr_storage *bound, int family )
   if (tcp_default_ip_addr(bound, family) < 0)
     return -1;
   if (bound->ss_family == AF_INET)
-    IP_AS_V4(*bound, port) = port;
+    IP_AS_V4(bound, port) = port;
   else
-    IP_AS_V6(*bound, port) = port;
+    IP_AS_V6(bound, port) = port;
   return 0;
 }
 

diff --git a/src/tcp.h b/src/tcp.h
index 726dfaeba31ed136797967d02a9ff68e0600742e..076f585a53758972e29d497cb402b4f766de7316 100644 (file)
--- a/src/tcp.h
+++ b/src/tcp.h
@@ -26,8 +26,8 @@
 #include <sys/socket.h>
 #endif
 
-#define IP_AS_V4(storage, f) ((struct sockaddr_in *)&(storage))->sin_##f
-#define IP_AS_V6(storage, f) ((struct sockaddr_in6 *)&(storage))->sin6_##f
+#define IP_AS_V4(storage, f) ((struct sockaddr_in *)(storage))->sin_##f
+#define IP_AS_V6(storage, f) ((struct sockaddr_in6 *)(storage))->sin6_##f
 #define IP_IN_ADDR(storage) \
   ((storage).ss_family == AF_INET6 ? \
       &((struct sockaddr_in6 *)&(storage))->sin6_addr : \

diff --git a/src/tvhlog.c b/src/tvhlog.c
index ec0eba874f3448c3ecbcbe3c97a7b2a778d8f08b..82f565e2ddf8af686ade45e5ef946200b499d556 100644 (file)
--- a/src/tvhlog.c
+++ b/src/tvhlog.c
@@ -587,7 +587,7 @@ tvhlog_init ( int level, int options, const char *path )
     if (rtport > 0) {
       tvhlog_rtfd = tvh_socket(AF_INET, SOCK_DGRAM, 0);
       tcp_get_ip_from_str("127.0.0.1", &tvhlog_rtss);
-      IP_AS_V4(tvhlog_rtss, port) = htons(rtport);
+      IP_AS_V4(&tvhlog_rtss, port) = htons(rtport);
     }
   }
 #endif

diff --git a/src/udp.c b/src/udp.c
index b88734011a4a1a6e1c299fa1856a174978b3dfd4..b0c3c1e5ca588923e31eb5aa18ede901a94130e8 100644 (file)
--- a/src/udp.c
+++ b/src/udp.c
@@ -76,15 +76,15 @@ udp_resolve( udp_connection_t *uc,
   }
   if (use->ai_family == AF_INET6) {
     ss->ss_family        = AF_INET6;
-    IP_AS_V6(*ss, port)  = htons(port);
-    memcpy(&IP_AS_V6(*ss, addr), &((struct sockaddr_in6 *)use->ai_addr)->sin6_addr,
+    IP_AS_V6(ss, port)  = htons(port);
+    memcpy(&IP_AS_V6(ss, addr), &((struct sockaddr_in6 *)use->ai_addr)->sin6_addr,
                                                              sizeof(struct in6_addr));
-    *multicast           = !!IN6_IS_ADDR_MULTICAST(&IP_AS_V6(*ss, addr));
+    *multicast           = !!IN6_IS_ADDR_MULTICAST(&IP_AS_V6(ss, addr));
   } else if (use->ai_family == AF_INET) {
     ss->ss_family        = AF_INET;
-    IP_AS_V4(*ss, port)  = htons(port);
-    IP_AS_V4(*ss, addr)  = ((struct sockaddr_in *)use->ai_addr)->sin_addr;
-    *multicast           = !!IN_MULTICAST(ntohl(IP_AS_V4(*ss, addr.s_addr)));
+    IP_AS_V4(ss, port)  = htons(port);
+    IP_AS_V4(ss, addr)  = ((struct sockaddr_in *)use->ai_addr)->sin_addr;
+    *multicast           = !!IN_MULTICAST(ntohl(IP_AS_V4(ss, addr.s_addr)));
   }
   freeaddrinfo(ressave);
   if (ss->ss_family != AF_INET && ss->ss_family != AF_INET6) {
@@ -207,9 +207,9 @@ udp_bind ( int subsystem, const char *name,
     /* Bind useful for receiver subsystem (not for udp streamer) */
     if (subsystem != LS_UDP) {
     if (bind(fd, (struct sockaddr *)&uc->ip, sizeof(struct sockaddr_in))) {
-      inet_ntop(AF_INET, &IP_AS_V4(uc->ip, addr), buf, sizeof(buf));
+      inet_ntop(AF_INET, &IP_AS_V4(&uc->ip, addr), buf, sizeof(buf));
       tvherror(subsystem, "%s - cannot bind %s:%hu [e=%s]",
-               name, buf, ntohs(IP_AS_V4(uc->ip, port)), strerror(errno));
+               name, buf, ntohs(IP_AS_V4(&uc->ip, port)), strerror(errno));
       goto error;
     }
     }  
@@ -221,7 +221,7 @@ udp_bind ( int subsystem, const char *name,
         struct ip_mreq_source ms;
         memset(&ms, 0, sizeof(ms));
 
-        ms.imr_multiaddr = IP_AS_V4(uc->ip, addr);
+        ms.imr_multiaddr = IP_AS_V4(&uc->ip, addr);
 
         /* Note, ip_mreq_source does not support the ifindex parameter,
            so we have to resolve to the ip of the interface on all platforms. */
@@ -253,7 +253,7 @@ udp_bind ( int subsystem, const char *name,
 #endif
         memset(&m,   0, sizeof(m));
 
-        m.imr_multiaddr      = IP_AS_V4(uc->ip, addr);
+        m.imr_multiaddr      = IP_AS_V4(&uc->ip, addr);
 #if !defined(PLATFORM_DARWIN)
         m.imr_address.s_addr = 0;
         m.imr_ifindex        = ifindex;
@@ -280,15 +280,15 @@ udp_bind ( int subsystem, const char *name,
 
     /* Bind */
     if (bind(fd, (struct sockaddr *)&uc->ip, sizeof(struct sockaddr_in6))) {
-      inet_ntop(AF_INET6, &IP_AS_V6(uc->ip, addr), buf, sizeof(buf));
+      inet_ntop(AF_INET6, &IP_AS_V6(&uc->ip, addr), buf, sizeof(buf));
       tvherror(subsystem, "%s - cannot bind %s:%hu [e=%s]",
-               name, buf, ntohs(IP_AS_V6(uc->ip, port)), strerror(errno));
+               name, buf, ntohs(IP_AS_V6(&uc->ip, port)), strerror(errno));
       goto error;
     }
 
     if (uc->multicast) {
       /* Join group */
-      m.ipv6mr_multiaddr = IP_AS_V6(uc->ip, addr);
+      m.ipv6mr_multiaddr = IP_AS_V6(&uc->ip, addr);
       m.ipv6mr_interface = ifindex;
 #ifdef SOL_IPV6
       if (setsockopt(fd, SOL_IPV6, IPV6_ADD_MEMBERSHIP, &m, sizeof(m))) {

diff --git a/src/upnp.c b/src/upnp.c
index 8842aab32f4329e72aa26d436373dd7a4880012f..f6a3c649f1eb3760604f0c1068116776dc296732 100644 (file)
--- a/src/upnp.c
+++ b/src/upnp.c
@@ -227,8 +227,8 @@ upnp_server_init(const char *bindaddr)
 
   memset(&upnp_ipv4_multicast, 0, sizeof(upnp_ipv4_multicast));
   upnp_ipv4_multicast.ss_family       = AF_INET;
-  IP_AS_V4(upnp_ipv4_multicast, port) = htons(1900);
-  r = inet_pton(AF_INET, "239.255.255.250", &IP_AS_V4(upnp_ipv4_multicast, addr));
+  IP_AS_V4(&upnp_ipv4_multicast, port) = htons(1900);
+  r = inet_pton(AF_INET, "239.255.255.250", &IP_AS_V4(&upnp_ipv4_multicast, addr));
   assert(r);
 
   tvh_mutex_init(&upnp_lock, NULL);