Assume that SOCK_CLOEXEC is available and works
diff mbox

Message ID 561CF8CC.3030608@redhat.com
State New
Headers show

Commit Message

Florian Weimer Oct. 13, 2015, 12:27 p.m. UTC
This fixes (harmless, I assume) data races when accessing the various
__have_sock_cloexec variables.

Compiled and tested on x86_64-redhat-linux-gnu.  According to the
previous thread (“O_CLOEXEC and SOCK_CLOEXEC“), there might be some
impact of this change on Hurd and NaCl, but it seems manageable.

Florian

Comments

Mike Frysinger Oct. 17, 2015, 3:25 a.m. UTC | #1
On 13 Oct 2015 14:27, Florian Weimer wrote:
> This fixes (harmless, I assume) data races when accessing the various
> __have_sock_cloexec variables.
> 
> Compiled and tested on x86_64-redhat-linux-gnu.  According to the
> previous thread (“O_CLOEXEC and SOCK_CLOEXEC“), there might be some
> impact of this change on Hurd and NaCl, but it seems manageable.

looks good, but shouldn't we also drop __ASSUME_O_CLOEXEC ?
-mike
Florian Weimer Oct. 17, 2015, 5:39 a.m. UTC | #2
* Mike Frysinger:

> On 13 Oct 2015 14:27, Florian Weimer wrote:
>> This fixes (harmless, I assume) data races when accessing the various
>> __have_sock_cloexec variables.
>> 
>> Compiled and tested on x86_64-redhat-linux-gnu.  According to the
>> previous thread (“O_CLOEXEC and SOCK_CLOEXEC“), there might be some
>> impact of this change on Hurd and NaCl, but it seems manageable.
>
> looks good, but shouldn't we also drop __ASSUME_O_CLOEXEC ?

Thanks.  Yes, __ASSUME_O_CLOEXEC is next, but it seemed it would be a
much larger patch, so I wanted to test the waters first.

Patch
diff mbox

2015-10-13  Florian Weimer  <fweimer@redhat.com>

	* include/sys/socket.h (__have_sock_cloexec): Remove declaration.
	(__have_paccept): Remove unused macro.
	* include/unistd.h (__have_sock_cloexec): Remove declaration.
	* misc/syslog.c (openlog_internal): Remove fallback code for
	!__ASSUME_SOCK_CLOEXEC.
	* nis/ypclnt.c (yp_bind_client_create): Remove fallback code for
	missing SOCK_CLOEXEC.
	* nscd/connections.c (have_sock_cloexec): Remove definition.
	(nscd_init): Remove fallback code for !__ASSUME_SOCK_CLOEXEC.
	* nscd/nscd_helper.c (open_socket): Remove fallback code for
	!__ASSUME_SOCK_CLOEXEC.
	* resolv/res_send.c (__have_o_nonblock): Remove definition.
	(reopen): Remove fallback code for !__ASSUME_SOCK_CLOEXEC.
	* socket/have_sock_cloexec.c (__have_sock_cloexec): Remove
	definition.
	* sunrpc/clnt_udp.c (__libc_clntudp_bufcreate): Remove fallback
	code for !__ASSUME_SOCK_CLOEXEC.
	* sysdeps/unix/sysv/linux/kernel-features.h
	(__ASSUME_SOCK_CLOEXEC): Remove.

diff --git a/include/sys/socket.h b/include/sys/socket.h
index 2f4bfd3..a00ab3c 100644
--- a/include/sys/socket.h
+++ b/include/sys/socket.h
@@ -154,13 +154,5 @@  libc_hidden_proto (__libc_sa_len)
 # define SA_LEN(_x)      __libc_sa_len((_x)->sa_family)
 #endif
 
-#ifdef SOCK_CLOEXEC
-extern int __have_sock_cloexec attribute_hidden;
-/* At lot of other functionality became available at the same time as
-   SOCK_CLOEXEC.  Avoid defining separate variables for all of them
-   unless it is really necessary.  */
-# define __have_paccept __have_sock_cloexec
-#endif
-
 #endif
 #endif
diff --git a/include/unistd.h b/include/unistd.h
index fbba393..cb41637 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -170,7 +170,6 @@  extern int __libc_pause (void);
 /* Not cancelable variant.  */
 extern int __pause_nocancel (void) attribute_hidden;
 
-extern int __have_sock_cloexec attribute_hidden;
 extern int __have_pipe2 attribute_hidden;
 extern int __have_dup3 attribute_hidden;
 
diff --git a/misc/syslog.c b/misc/syslog.c
index 034e2c8..9e3d34d 100644
--- a/misc/syslog.c
+++ b/misc/syslog.c
@@ -346,36 +346,9 @@  openlog_internal(const char *ident, int logstat, int logfac)
 			(void)strncpy(SyslogAddr.sun_path, _PATH_LOG,
 				      sizeof(SyslogAddr.sun_path));
 			if (LogStat & LOG_NDELAY) {
-#ifdef SOCK_CLOEXEC
-# ifndef __ASSUME_SOCK_CLOEXEC
-				if (__have_sock_cloexec >= 0) {
-# endif
-					LogFile = __socket(AF_UNIX,
-							   LogType
-							   | SOCK_CLOEXEC, 0);
-# ifndef __ASSUME_SOCK_CLOEXEC
-					if (__have_sock_cloexec == 0)
-						__have_sock_cloexec
-						  = ((LogFile != -1
-						      || errno != EINVAL)
-						     ? 1 : -1);
-				}
-# endif
-#endif
-#ifndef __ASSUME_SOCK_CLOEXEC
-# ifdef SOCK_CLOEXEC
-				if (__have_sock_cloexec < 0)
-# endif
-				  LogFile = __socket(AF_UNIX, LogType, 0);
-#endif
-				if (LogFile == -1)
-					return;
-#ifndef __ASSUME_SOCK_CLOEXEC
-# ifdef SOCK_CLOEXEC
-				if (__have_sock_cloexec < 0)
-# endif
-					__fcntl(LogFile, F_SETFD, FD_CLOEXEC);
-#endif
+			  LogFile = __socket(AF_UNIX, LogType | SOCK_CLOEXEC, 0);
+			  if (LogFile == -1)
+			    return;
 			}
 		}
 		if (LogFile != -1 && !connected)
diff --git a/nis/ypclnt.c b/nis/ypclnt.c
index 3a73872..93a95d6 100644
--- a/nis/ypclnt.c
+++ b/nis/ypclnt.c
@@ -68,25 +68,11 @@  yp_bind_client_create (const char *domain, dom_binding *ysd,
   ysd->dom_domain[YPMAXDOMAIN] = '\0';
 
   ysd->dom_socket = RPC_ANYSOCK;
-#ifdef SOCK_CLOEXEC
-# define xflags SOCK_CLOEXEC
-#else
-# define xflags 0
-#endif
   ysd->dom_client = __libc_clntudp_bufcreate (&ysd->dom_server_addr, YPPROG,
 					      YPVERS, UDPTIMEOUT,
 					      &ysd->dom_socket,
 					      UDPMSGSIZE, UDPMSGSIZE,
-					      xflags);
-
-  if (ysd->dom_client != NULL)
-    {
-#ifndef SOCK_CLOEXEC
-      /* If the program exits, close the socket */
-      if (fcntl (ysd->dom_socket, F_SETFD, FD_CLOEXEC) == -1)
-	perror ("fcntl: F_SETFD");
-#endif
-    }
+					      SOCK_CLOEXEC);
 }
 
 #if USE_BINDINGDIR
diff --git a/nscd/connections.c b/nscd/connections.c
index cba5e6a..e16406b 100644
--- a/nscd/connections.c
+++ b/nscd/connections.c
@@ -257,11 +257,6 @@  int inotify_fd = -1;
 static int nl_status_fd = -1;
 #endif
 
-#ifndef __ASSUME_SOCK_CLOEXEC
-/* Negative if SOCK_CLOEXEC is not supported, positive if it is, zero
-   before be know the result.  */
-static int have_sock_cloexec;
-#endif
 #ifndef __ASSUME_ACCEPT4
 static int have_accept4;
 #endif
@@ -830,21 +825,7 @@  cannot set socket to close on exec: %s; disabling paranoia mode"),
       }
 
   /* Create the socket.  */
-#ifndef __ASSUME_SOCK_CLOEXEC
-  sock = -1;
-  if (have_sock_cloexec >= 0)
-#endif
-    {
-      sock = socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
-#ifndef __ASSUME_SOCK_CLOEXEC
-      if (have_sock_cloexec == 0)
-	have_sock_cloexec = sock != -1 || errno != EINVAL ? 1 : -1;
-#endif
-    }
-#ifndef __ASSUME_SOCK_CLOEXEC
-  if (have_sock_cloexec < 0)
-    sock = socket (AF_UNIX, SOCK_STREAM, 0);
-#endif
+  sock = socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
   if (sock < 0)
     {
       dbg_log (_("cannot open socket: %s"), strerror (errno));
@@ -860,28 +841,6 @@  cannot set socket to close on exec: %s; disabling paranoia mode"),
       do_exit (errno == EACCES ? 4 : 1, 0, NULL);
     }
 
-#ifndef __ASSUME_SOCK_CLOEXEC
-  if (have_sock_cloexec < 0)
-    {
-      /* We don't want to get stuck on accept.  */
-      int fl = fcntl (sock, F_GETFL);
-      if (fl == -1 || fcntl (sock, F_SETFL, fl | O_NONBLOCK) == -1)
-	{
-	  dbg_log (_("cannot change socket to nonblocking mode: %s"),
-		   strerror (errno));
-	  do_exit (1, 0, NULL);
-	}
-
-      /* The descriptor needs to be closed on exec.  */
-      if (paranoia && fcntl (sock, F_SETFD, FD_CLOEXEC) == -1)
-	{
-	  dbg_log (_("cannot set socket to close on exec: %s"),
-		   strerror (errno));
-	  do_exit (1, 0, NULL);
-	}
-    }
-#endif
-
   /* Set permissions for the socket.  */
   chmod (_PATH_NSCDSOCKET, DEFFILEMODE);
 
@@ -922,31 +881,6 @@  cannot set socket to close on exec: %s; disabling paranoia mode"),
 	      /* Start the timestamp process.  */
 	      dbs[hstdb].head->extra_data[NSCD_HST_IDX_CONF_TIMESTAMP]
 		= __bump_nl_timestamp ();
-
-# ifndef __ASSUME_SOCK_CLOEXEC
-	      if (have_sock_cloexec < 0)
-		{
-		  /* We don't want to get stuck on accept.  */
-		  int fl = fcntl (nl_status_fd, F_GETFL);
-		  if (fl == -1
-		      || fcntl (nl_status_fd, F_SETFL, fl | O_NONBLOCK) == -1)
-		    {
-		      dbg_log (_("\
-cannot change socket to nonblocking mode: %s"),
-			       strerror (errno));
-		      do_exit (1, 0, NULL);
-		    }
-
-		  /* The descriptor needs to be closed on exec.  */
-		  if (paranoia
-		      && fcntl (nl_status_fd, F_SETFD, FD_CLOEXEC) == -1)
-		    {
-		      dbg_log (_("cannot set socket to close on exec: %s"),
-			       strerror (errno));
-		      do_exit (1, 0, NULL);
-		    }
-		}
-# endif
 	    }
 	}
     }
diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
index 52a5caa..341b931 100644
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -166,24 +166,7 @@  open_socket (request_type type, const char *key, size_t keylen)
 {
   int sock;
 
-#ifdef SOCK_CLOEXEC
-# ifndef __ASSUME_SOCK_CLOEXEC
-  if (__have_sock_cloexec >= 0)
-# endif
-    {
-      sock = __socket (PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
-# ifndef __ASSUME_SOCK_CLOEXEC
-      if (__have_sock_cloexec == 0)
-	__have_sock_cloexec = sock != -1 || errno != EINVAL ? 1 : -1;
-# endif
-    }
-#endif
-#ifndef __ASSUME_SOCK_CLOEXEC
-# ifdef SOCK_CLOEXEC
-  if (__have_sock_cloexec < 0)
-# endif
-    sock = __socket (PF_UNIX, SOCK_STREAM, 0);
-#endif
+  sock = __socket (PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
   if (sock < 0)
     return -1;
 
@@ -194,14 +177,6 @@  open_socket (request_type type, const char *key, size_t keylen)
     char key[];
   } *reqdata = alloca (real_sizeof_reqdata);
 
-#ifndef __ASSUME_SOCK_CLOEXEC
-# ifdef SOCK_NONBLOCK
-  if (__have_sock_cloexec < 0)
-# endif
-    /* Make socket non-blocking.  */
-    __fcntl (sock, F_SETFL, O_RDWR | O_NONBLOCK);
-#endif
-
   struct sockaddr_un sun;
   sun.sun_family = AF_UNIX;
   strcpy (sun.sun_path, _PATH_NSCDSOCKET);
diff --git a/resolv/res_send.c b/resolv/res_send.c
index 5e53cc2..6137e4d 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -104,14 +104,6 @@  static const char rcsid[] = "$BINDId: res_send.c,v 8.38 2000/03/30 20:16:51 vixi
 #define MAXPACKET       65536
 #endif
 
-
-#ifndef __ASSUME_SOCK_CLOEXEC
-static int __have_o_nonblock;
-#else
-# define __have_o_nonblock 0
-#endif
-
-
 /* From ev_streams.c.  */
 
 static inline void
@@ -927,38 +919,14 @@  reopen (res_state statp, int *terrno, int ns)
 
 		/* only try IPv6 if IPv6 NS and if not failed before */
 		if (nsap->sa_family == AF_INET6 && !statp->ipv6_unavail) {
-			if (__glibc_likely (__have_o_nonblock >= 0))       {
-				EXT(statp).nssocks[ns] =
-				  socket(PF_INET6, SOCK_DGRAM|SOCK_NONBLOCK,
-					 0);
-#ifndef __ASSUME_SOCK_CLOEXEC
-				if (__have_o_nonblock == 0)
-					__have_o_nonblock
-					  = (EXT(statp).nssocks[ns] == -1
-					     && errno == EINVAL ? -1 : 1);
-#endif
-			}
-			if (__glibc_unlikely (__have_o_nonblock < 0))
-				EXT(statp).nssocks[ns] =
-				  socket(PF_INET6, SOCK_DGRAM, 0);
+			EXT(statp).nssocks[ns]
+				= socket(PF_INET6, SOCK_DGRAM|SOCK_NONBLOCK, 0);
 			if (EXT(statp).nssocks[ns] < 0)
 			    statp->ipv6_unavail = errno == EAFNOSUPPORT;
 			slen = sizeof (struct sockaddr_in6);
 		} else if (nsap->sa_family == AF_INET) {
-			if (__glibc_likely (__have_o_nonblock >= 0))       {
-				EXT(statp).nssocks[ns]
-				  = socket(PF_INET, SOCK_DGRAM|SOCK_NONBLOCK,
-					   0);
-#ifndef __ASSUME_SOCK_CLOEXEC
-				if (__have_o_nonblock == 0)
-					__have_o_nonblock
-					  = (EXT(statp).nssocks[ns] == -1
-					     && errno == EINVAL ? -1 : 1);
-#endif
-			}
-			if (__glibc_unlikely (__have_o_nonblock < 0))
-				EXT(statp).nssocks[ns]
-				  = socket(PF_INET, SOCK_DGRAM, 0);
+			EXT(statp).nssocks[ns]
+				= socket(PF_INET, SOCK_DGRAM|SOCK_NONBLOCK, 0);
 			slen = sizeof (struct sockaddr_in);
 		}
 		if (EXT(statp).nssocks[ns] < 0) {
@@ -983,15 +951,6 @@  reopen (res_state statp, int *terrno, int ns)
 			__res_iclose(statp, false);
 			return (0);
 		}
-		if (__glibc_unlikely (__have_o_nonblock < 0))       {
-			/* Make socket non-blocking.  */
-			int fl = __fcntl (EXT(statp).nssocks[ns], F_GETFL);
-			if  (fl != -1)
-				__fcntl (EXT(statp).nssocks[ns], F_SETFL,
-					 fl | O_NONBLOCK);
-			Dprint(statp->options & RES_DEBUG,
-			       (stdout, ";; new DG socket\n"))
-		}
 	}
 
 	return 1;
diff --git a/socket/have_sock_cloexec.c b/socket/have_sock_cloexec.c
index c849f92..72b184c 100644
--- a/socket/have_sock_cloexec.c
+++ b/socket/have_sock_cloexec.c
@@ -19,10 +19,6 @@ 
 #include <sys/socket.h>
 #include <kernel-features.h>
 
-#if defined SOCK_CLOEXEC && !defined __ASSUME_SOCK_CLOEXEC
-int __have_sock_cloexec;
-#endif
-
 #if defined O_CLOEXEC && !defined __ASSUME_PIPE2
 int __have_pipe2;
 #endif
diff --git a/sunrpc/clnt_udp.c b/sunrpc/clnt_udp.c
index 6ffa5f2..cc1da47 100644
--- a/sunrpc/clnt_udp.c
+++ b/sunrpc/clnt_udp.c
@@ -171,31 +171,7 @@  __libc_clntudp_bufcreate (struct sockaddr_in *raddr, u_long program,
   cu->cu_xdrpos = XDR_GETPOS (&(cu->cu_outxdrs));
   if (*sockp < 0)
     {
-#ifdef SOCK_NONBLOCK
-# ifndef __ASSUME_SOCK_CLOEXEC
-      if (__have_sock_cloexec >= 0)
-# endif
-	{
-	  *sockp = __socket (AF_INET, SOCK_DGRAM|SOCK_NONBLOCK|flags,
-			     IPPROTO_UDP);
-# ifndef __ASSUME_SOCK_CLOEXEC
-	  if (__have_sock_cloexec == 0)
-	    __have_sock_cloexec = *sockp >= 0 || errno != EINVAL ? 1 : -1;
-# endif
-	}
-#endif
-#ifndef __ASSUME_SOCK_CLOEXEC
-# ifdef SOCK_CLOEXEC
-      if (__have_sock_cloexec < 0)
-# endif
-	{
-	  *sockp = __socket (AF_INET, SOCK_DGRAM, IPPROTO_UDP);
-# ifdef SOCK_CLOEXEC
-	  if (flags & SOCK_CLOEXEC)
-	    __fcntl (*sockp, F_SETFD, FD_CLOEXEC);
-# endif
-	}
-#endif
+      *sockp = __socket (AF_INET, SOCK_DGRAM|SOCK_NONBLOCK|flags, IPPROTO_UDP);
       if (__glibc_unlikely (*sockp < 0))
 	{
 	  struct rpc_createerr *ce = &get_rpc_createerr ();
@@ -205,16 +181,6 @@  __libc_clntudp_bufcreate (struct sockaddr_in *raddr, u_long program,
 	}
       /* attempt to bind to prov port */
       (void) bindresvport (*sockp, (struct sockaddr_in *) 0);
-#ifndef __ASSUME_SOCK_CLOEXEC
-# ifdef SOCK_CLOEXEC
-      if (__have_sock_cloexec < 0)
-# endif
-	{
-	  /* the sockets rpc controls are non-blocking */
-	  int dontblock = 1;
-	  (void) __ioctl (*sockp, FIONBIO, (char *) &dontblock);
-	}
-#endif
 #ifdef IP_RECVERR
       {
 	int on = 1;
diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
index ce127d6..feb63bb 100644
--- a/sysdeps/unix/sysv/linux/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/kernel-features.h
@@ -86,7 +86,6 @@ 
 
 /* Support for various CLOEXEC and NONBLOCK flags was added in
    2.6.27.  */
-#define __ASSUME_SOCK_CLOEXEC	1
 #define __ASSUME_IN_NONBLOCK	1
 #define __ASSUME_PIPE2		1
 #define __ASSUME_EVENTFD2	1