This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[pushed] Fix struct sockaddr/sockaddr_in/sockaddr_un strict aliasing violations


Building gdbserver in C++ mode shows:

  gdb/gdbserver/tracepoint.c: In function âvoid* gdb_agent_helper_thread(void*)â:
  gdb/gdbserver/tracepoint.c:7190:47: error: cannot convert âsockaddr_un*â to âsockaddr*â for argument â2â to âint accept(int, sockaddr*, socklen_t*)â
	  fd = accept (listen_fd, &sockaddr, &tmp);

A few places in the tree already have an explicit cast to struct
sockaddr *, but that's a strict aliasing violation.  Instead of
propagating invalid code, fix this by using a union instead.

gdb/ChangeLog:
2015-03-07  Pedro Alves  <palves@redhat.com>

	* common/gdb_socket.h: New file.
	* ser-tcp.c: Include gdb_socket.h.  Don't include netinet/in.h nor
	sys/socket.h.
	(net_open): Use union gdb_sockaddr_u.

gdb/gdbserver/ChangeLog:
2015-03-07  Pedro Alves  <palves@redhat.com>

	* gdbreplay.c: No longer include <netinet/in.h>, <sys/socket.h>,
	or <winsock2.h> here.  Instead include "gdb_socket.h".
	(remote_open): Use union gdb_sockaddr_u.
	* remote-utils.c: No longer include <netinet/in.h>, <sys/socket.h>
	or <winsock2.h> here.  Instead include "gdb_socket.h".
	(handle_accept_event, remote_prepare): Use union gdb_sockaddr_u.
	* tracepoint.c: Include "gdb_socket.h" instead of <sys/socket.h>
	or <sys/un.h>.
	(init_named_socket, gdb_agent_helper_thread): Use union
	gdb_sockaddr_u.
---
 gdb/ChangeLog                |  7 +++++++
 gdb/gdbserver/ChangeLog      | 13 +++++++++++++
 gdb/common/gdb_socket.h      | 43 +++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/gdbreplay.c    | 24 ++++++++----------------
 gdb/gdbserver/remote-utils.c | 29 ++++++++++-------------------
 gdb/gdbserver/tracepoint.c   | 19 +++++++++----------
 gdb/ser-tcp.c                | 14 +++++++-------
 7 files changed, 97 insertions(+), 52 deletions(-)
 create mode 100644 gdb/common/gdb_socket.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5da3f3..522f2bb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2015-03-07  Pedro Alves  <palves@redhat.com>
 
+	* common/gdb_socket.h: New file.
+	* ser-tcp.c: Include gdb_socket.h.  Don't include netinet/in.h nor
+	sys/socket.h.
+	(net_open): Use union gdb_sockaddr_u.
+
+2015-03-07  Pedro Alves  <palves@redhat.com>
+
 	* common/common-exceptions.c [!__cplusplus] (enum catcher_state)
 	(exceptions_state_mc_action_iter)
 	(exceptions_state_mc_action_iter_1, exceptions_state_mc_catch):
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 70cc374..8aa38a3 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,18 @@
 2015-03-07  Pedro Alves  <palves@redhat.com>
 
+	* gdbreplay.c: No longer include <netinet/in.h>, <sys/socket.h>,
+	or <winsock2.h> here.  Instead include "gdb_socket.h".
+	(remote_open): Use union gdb_sockaddr_u.
+	* remote-utils.c: No longer include <netinet/in.h>, <sys/socket.h>
+	or <winsock2.h> here.  Instead include "gdb_socket.h".
+	(handle_accept_event, remote_prepare): Use union gdb_sockaddr_u.
+	* tracepoint.c: Include "gdb_socket.h" instead of <sys/socket.h>
+	or <sys/un.h>.
+	(init_named_socket, gdb_agent_helper_thread): Use union
+	gdb_sockaddr_u.
+
+2015-03-07  Pedro Alves  <palves@redhat.com>
+
 	Adjust all callers of TRY_CATCH to use TRY/CATCH/END_CATCH
 	instead.
 
diff --git a/gdb/common/gdb_socket.h b/gdb/common/gdb_socket.h
new file mode 100644
index 0000000..a670f74
--- /dev/null
+++ b/gdb/common/gdb_socket.h
@@ -0,0 +1,43 @@
+/* Copyright (C) 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_SOCKET_H
+#define GDB_SOCKET_H
+
+#if USE_WIN32API
+#include <winsock2.h>
+#else
+#include <sys/socket.h>
+#include <netinet/in.h>
+#if HAVE_SYS_UN_H
+#include <sys/un.h>
+#endif
+#endif
+
+/* Use this union instead of casts between struct sockaddr <-> struct
+   sockaddr_foo to avoid strict aliasing violations.  */
+
+union gdb_sockaddr_u
+{
+  struct sockaddr sa;
+  struct sockaddr_in sa_in;
+#if HAVE_SYS_UN_H
+  struct sockaddr_un sa_un;
+#endif
+};
+
+#endif /* GDB_SOCKET_H */
diff --git a/gdb/gdbserver/gdbreplay.c b/gdb/gdbserver/gdbreplay.c
index a02a824..bfd6f19 100644
--- a/gdb/gdbserver/gdbreplay.c
+++ b/gdb/gdbserver/gdbreplay.c
@@ -36,24 +36,16 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#ifdef HAVE_NETINET_IN_H
-#include <netinet/in.h>
-#endif
-#ifdef HAVE_SYS_SOCKET_H
-#include <sys/socket.h>
-#endif
 #if HAVE_NETDB_H
 #include <netdb.h>
 #endif
 #if HAVE_NETINET_TCP_H
 #include <netinet/tcp.h>
 #endif
+#include "gdb_socket.h"
 
 #include <alloca.h>
 
-#if USE_WIN32API
-#include <winsock2.h>
-#endif
 
 #ifndef HAVE_SOCKLEN_T
 typedef int socklen_t;
@@ -188,7 +180,7 @@ remote_open (char *name)
 #endif
       char *port_str;
       int port;
-      struct sockaddr_in sockaddr;
+      union gdb_sockaddr_u sockaddr;
       socklen_t tmp;
       int tmp_desc;
 
@@ -215,16 +207,16 @@ remote_open (char *name)
       setsockopt (tmp_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
 		  sizeof (tmp));
 
-      sockaddr.sin_family = PF_INET;
-      sockaddr.sin_port = htons (port);
-      sockaddr.sin_addr.s_addr = INADDR_ANY;
+      sockaddr.sa_in.sin_family = PF_INET;
+      sockaddr.sa_in.sin_port = htons (port);
+      sockaddr.sa_in.sin_addr.s_addr = INADDR_ANY;
 
-      if (bind (tmp_desc, (struct sockaddr *) &sockaddr, sizeof (sockaddr))
+      if (bind (tmp_desc, &sockaddr.sa, sizeof (sockaddr.sa_in))
 	  || listen (tmp_desc, 1))
 	perror_with_name ("Can't bind address");
 
-      tmp = sizeof (sockaddr);
-      remote_desc = accept (tmp_desc, (struct sockaddr *) &sockaddr, &tmp);
+      tmp = sizeof (sockaddr.sa_in);
+      remote_desc = accept (tmp_desc, &sockaddr.sa, &tmp);
       if (remote_desc == -1)
 	perror_with_name ("Accept failed");
 
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1de86be..69f87bd 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -30,12 +30,6 @@
 #if HAVE_SYS_FILE_H
 #include <sys/file.h>
 #endif
-#if HAVE_NETINET_IN_H
-#include <netinet/in.h>
-#endif
-#if HAVE_SYS_SOCKET_H
-#include <sys/socket.h>
-#endif
 #if HAVE_NETDB_H
 #include <netdb.h>
 #endif
@@ -57,10 +51,7 @@
 #include <arpa/inet.h>
 #endif
 #include <sys/stat.h>
-
-#if USE_WIN32API
-#include <winsock2.h>
-#endif
+#include "gdb_socket.h"
 
 #if __QNX__
 #include <sys/iomgr.h>
@@ -153,14 +144,14 @@ enable_async_notification (int fd)
 static int
 handle_accept_event (int err, gdb_client_data client_data)
 {
-  struct sockaddr_in sockaddr;
+  union gdb_sockaddr_u sockaddr;
   socklen_t tmp;
 
   if (debug_threads)
     debug_printf ("handling possible accept event\n");
 
-  tmp = sizeof (sockaddr);
-  remote_desc = accept (listen_desc, (struct sockaddr *) &sockaddr, &tmp);
+  tmp = sizeof (sockaddr.sa_in);
+  remote_desc = accept (listen_desc, &sockaddr.sa, &tmp);
   if (remote_desc == -1)
     perror_with_name ("Accept failed");
 
@@ -195,7 +186,7 @@ handle_accept_event (int err, gdb_client_data client_data)
 
   /* Convert IP address to string.  */
   fprintf (stderr, "Remote debugging from host %s\n",
-	   inet_ntoa (sockaddr.sin_addr));
+	   inet_ntoa (sockaddr.sa_in.sin_addr));
 
   enable_async_notification (remote_desc);
 
@@ -224,7 +215,7 @@ remote_prepare (char *name)
   static int winsock_initialized;
 #endif
   int port;
-  struct sockaddr_in sockaddr;
+  union gdb_sockaddr_u sockaddr;
   socklen_t tmp;
   char *port_end;
 
@@ -269,11 +260,11 @@ remote_prepare (char *name)
   setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
 	      sizeof (tmp));
 
-  sockaddr.sin_family = PF_INET;
-  sockaddr.sin_port = htons (port);
-  sockaddr.sin_addr.s_addr = INADDR_ANY;
+  sockaddr.sa_in.sin_family = PF_INET;
+  sockaddr.sa_in.sin_port = htons (port);
+  sockaddr.sa_in.sin_addr.s_addr = INADDR_ANY;
 
-  if (bind (listen_desc, (struct sockaddr *) &sockaddr, sizeof (sockaddr))
+  if (bind (listen_desc, &sockaddr.sa, sizeof (sockaddr.sa_in))
       || listen (listen_desc, 1))
     perror_with_name ("Can't bind address");
 
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 27fcf03..6c34ad4 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -6817,8 +6817,7 @@ run_inferior_command (char *cmd, int len)
 
 #else /* !IN_PROCESS_AGENT */
 
-#include <sys/socket.h>
-#include <sys/un.h>
+#include "gdb_socket.h"
 
 #ifndef UNIX_PATH_MAX
 #define UNIX_PATH_MAX sizeof(((struct sockaddr_un *) NULL)->sun_path)
@@ -6837,7 +6836,7 @@ static int
 init_named_socket (const char *name)
 {
   int result, fd;
-  struct sockaddr_un addr;
+  union gdb_sockaddr_u addr;
 
   result = fd = socket (PF_UNIX, SOCK_STREAM, 0);
   if (result == -1)
@@ -6846,10 +6845,10 @@ init_named_socket (const char *name)
       return -1;
     }
 
-  addr.sun_family = AF_UNIX;
+  addr.sa_un.sun_family = AF_UNIX;
 
-  strncpy (addr.sun_path, name, UNIX_PATH_MAX);
-  addr.sun_path[UNIX_PATH_MAX - 1] = '\0';
+  strncpy (addr.sa_un.sun_path, name, UNIX_PATH_MAX);
+  addr.sa_un.sun_path[UNIX_PATH_MAX - 1] = '\0';
 
   result = access (name, F_OK);
   if (result == 0)
@@ -6865,7 +6864,7 @@ init_named_socket (const char *name)
       warning ("socket %s already exists; overwriting", name);
     }
 
-  result = bind (fd, (struct sockaddr *) &addr, sizeof (addr));
+  result = bind (fd, &addr.sa, sizeof (addr.sa_un));
   if (result == -1)
     {
       warning ("bind failed: %s", strerror (errno));
@@ -7164,17 +7163,17 @@ gdb_agent_helper_thread (void *arg)
       while (1)
 	{
 	  socklen_t tmp;
-	  struct sockaddr_un sockaddr;
+	  union gdb_sockaddr_u sockaddr;
 	  int fd;
 	  char buf[1];
 	  int ret;
 	  int stop_loop = 0;
 
-	  tmp = sizeof (sockaddr);
+	  tmp = sizeof (sockaddr.sa_un);
 
 	  do
 	    {
-	      fd = accept (listen_fd, &sockaddr, &tmp);
+	      fd = accept (listen_fd, &sockaddr.sa, &tmp);
 	    }
 	  /* It seems an ERESTARTSYS can escape out of accept.  */
 	  while (fd == -512 || (fd == -1 && errno == EINTR));
diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index 9c3dcf4..648f1fb 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -37,6 +37,8 @@
 
 #include <sys/time.h>
 
+#include "gdb_socket.h"
+
 #ifdef USE_WIN32API
 #include <winsock2.h>
 #ifndef ETIMEDOUT
@@ -45,10 +47,8 @@
 #define close(fd) closesocket (fd)
 #define ioctl ioctlsocket
 #else
-#include <netinet/in.h>
 #include <arpa/inet.h>
 #include <netdb.h>
-#include <sys/socket.h>
 #include <netinet/tcp.h>
 #endif
 
@@ -159,7 +159,7 @@ net_open (struct serial *scb, const char *name)
   int n, port, tmp;
   int use_udp;
   struct hostent *hostent;
-  struct sockaddr_in sockaddr;
+  union gdb_sockaddr_u sockaddr;
 #ifdef USE_WIN32API
   u_long ioarg;
 #else
@@ -199,9 +199,9 @@ net_open (struct serial *scb, const char *name)
       return -1;
     }
 
-  sockaddr.sin_family = PF_INET;
-  sockaddr.sin_port = htons (port);
-  memcpy (&sockaddr.sin_addr.s_addr, hostent->h_addr,
+  sockaddr.sa_in.sin_family = PF_INET;
+  sockaddr.sa_in.sin_port = htons (port);
+  memcpy (&sockaddr.sa_in.sin_addr.s_addr, hostent->h_addr,
 	  sizeof (struct in_addr));
 
  retry:
@@ -220,7 +220,7 @@ net_open (struct serial *scb, const char *name)
 
   /* Use Non-blocking connect.  connect() will return 0 if connected
      already.  */
-  n = connect (scb->fd, (struct sockaddr *) &sockaddr, sizeof (sockaddr));
+  n = connect (scb->fd, &sockaddr.sa, sizeof (sockaddr.sa_in));
 
   if (n < 0)
     {
-- 
1.9.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]