RFC on enforcing best-practice through wrappers? [PR15819, PR15722]
diff mbox

Message ID oroasm6pw9.fsf@free.home
State New
Headers show

Commit Message

Alexandre Oliva Nov. 5, 2014, 7:57 a.m. UTC
These two PRs are about unrelated issues, but they share a common theme:
standard APIs do not follow best practice by default, our code often
fails to jump through the required hoops to do so and, even if it did,
it would be best to factor out what it takes to follow best practice
into a single place, and use it all over, perhaps even by default.

The two cases at hand are PR15819 (poll EINTRs and we retry without
adjusting the timeout) and PR15722 (sockets are created without
CLOEXEC).

There are various ways to go about fixing this, ranging from using
__poll and __socket internal wrappers/aliases to avoid the undesirable
problems in the first place (say, having __poll never return EINTR, but
retrying with a recomputed timeout instead instead, and having __socket
always set CLOEXEC), introducing alternate functions to do so and
perhaps poisoning the original symbols so that misuses are flagged, up
to just offering wrappers that implement the best practice in internal
header wrappers or new internal headers.  AFAICT we don't seem to have
any recommended practice on how to factor out, let alone internally
enforce, this sort of best practice.

In these two patches, I've followed the last two approaches: one
introduces a static inline wrapper __poll_noeintr in sys/poll.h, that
will reenter the syscall with a recomputed timeout if it's interrupted,
and the other introduces a new header socket-cloexec.h that defines
__socket_cloexec and __socket_cloexec_nonblock static inline wrappers to
__socket that do what it takes to set these flags even when SOCK_CLOEXEC
and SOCK_NONBLOCK are not supported.

The reason for the differences is that I'd arranged for the headers to
include all headers required for functions using errno, gettimeofday,
fcntl, ioctl and whatnot to be properly parsed, and in the case of
__socket_cloexec, this became an include loop that caused some headers
to fail to be parsed correctly because referenced types hadn't been
defined after the nested #include of the header supposed to define them.

I didn't see much precedent for static inline functions in internal
headers, so I'm a bit hesitant to go either way and maybe set an
unintended precedent.  OTOH, using preprocessor macros, which would
alleviate the problem of #include loops because the macros would only be
parsed at expansion time, is more fragile to begin with, and there is
still a risk that mutually-dependent headers get included in the wrong
order and break stuff.  OYAH (On yet another hand ;-) using out-of-line
functions would introduce another layer of indirection between the
syscall and its client, which might be undesirable from a performance
perspective.

Any thoughts or recommendations on how we should address this sort of
issue, ideally in a way that makes it easier to follow best practice
than to forget to do so?

While at that, are these interfaces ones we might want to offer to our
users, as GNU extensions?

Is it overkill to offer the __socket_cloexec extra argument, on whether
or not CLOEXEC is mandatory and the whole thing should fail if we can't
make the socket cloexec?

Should __socket_cloexec_nonblock attempt a third way to make a socket
non-blocking, if the first two fail?  (absent SOCK_NONBLOCK, I'm trying
fcntl F_SETFL, like nscd, but I see sunrpc/clnt_udp.c uses the FIONBIO
ioctl)


Thanks in advance,


Here are patchlets that implement two of the alternatives I've
described, for reference.  Once we agree on one standard way to do this
sort of thing, I'll adjust the patches as needed and submit them
in a form more suitable for inclusion.

Comments

Joseph Myers Nov. 5, 2014, 4:45 p.m. UTC | #1
On Wed, 5 Nov 2014, Alexandre Oliva wrote:

> +      struct timeval __now;
> +      (void) __gettimeofday (&__now, NULL);
> +
> +      long int __end = __now.tv_sec * 1000 + __timeout
> +	+ (__now.tv_usec + 500) / 1000;

This multiplication by 1000 will overflow on 32-bit systems.  Things may 
in fact work OK if it wraps around, but it's not a good idea to rely on 
that without explicit casts to unsigned and comments about why it's OK 
even when things wrap around.  (Pre-existing condition in the code you're 
moving.)
Rich Felker Nov. 5, 2014, 6:40 p.m. UTC | #2
On Wed, Nov 05, 2014 at 04:45:21PM +0000, Joseph Myers wrote:
> On Wed, 5 Nov 2014, Alexandre Oliva wrote:
> 
> > +      struct timeval __now;
> > +      (void) __gettimeofday (&__now, NULL);
> > +
> > +      long int __end = __now.tv_sec * 1000 + __timeout
> > +	+ (__now.tv_usec + 500) / 1000;
> 
> This multiplication by 1000 will overflow on 32-bit systems.  Things may 
> in fact work OK if it wraps around, but it's not a good idea to rely on 
> that without explicit casts to unsigned and comments about why it's OK 
> even when things wrap around.  (Pre-existing condition in the code you're 
> moving.)

I suspect it's also semantically wrong. For emulating relative
timeouts you generally want the monotonic clock, not the realtime
clock.

Rich
Alexandre Oliva Nov. 6, 2014, 8:16 p.m. UTC | #3
On Nov  5, 2014, Rich Felker <dalias@libc.org> wrote:

> On Wed, Nov 05, 2014 at 04:45:21PM +0000, Joseph Myers wrote:
>> On Wed, 5 Nov 2014, Alexandre Oliva wrote:
>> 
>> > +      struct timeval __now;
>> > +      (void) __gettimeofday (&__now, NULL);
>> > +
>> > +      long int __end = __now.tv_sec * 1000 + __timeout
>> > +	+ (__now.tv_usec + 500) / 1000;
>> 
>> This multiplication by 1000 will overflow on 32-bit systems.  Things may 
>> in fact work OK if it wraps around, but it's not a good idea to rely on 
>> that without explicit casts to unsigned and comments about why it's OK 
>> even when things wrap around.  (Pre-existing condition in the code you're 
>> moving.)

> I suspect it's also semantically wrong. For emulating relative
> timeouts you generally want the monotonic clock, not the realtime
> clock.

Yup.  I suppose I might thank you both your for attentiveness to these
details, even though both issues had been noticed by yours truly.
However, since they were in code being moved by this patchlet, best
practice dictates that it be fixed in separate patches.

Besides, what my message called for was policy discussion, not review of
patchlets that were definitely not intended for inclusion in the
sourcebase yet.

So, anyone got comments on the policy issues, so that the patches can be
both finished and submitted for review, so that the bugs can be
ultimately fixed?

Thanks in advance,
Roland McGrath Nov. 6, 2014, 10:28 p.m. UTC | #4
Prefer inlines to macros.  Clean up header tangles and internal
arrangements as necessary to make it doable.  There is probably a lot to
unwind there and to just make progress quickly it will often be easier
to use a macro.  I'm sure there will be cases where we're happy enough
to just have new macro use because it makes things better without
waiting for a lot of hair-pulling header refactoring.  But it is the
right long-term direction to have fewer function-like macros when they
can be inline functions instead, and the effort put into the header
cleanups should pay dividends in terms of easier maintenance and
code-reading aside from the specific inline enablement motivating the work.

I think it would be better to move towards more internal-only headers
and less having extra magic in include/foo.h wrapper headers.  Ideally I
think those would only be for hidden_proto and the like--where it's not
changing the API offered by #include <foo.h> beyond __-prefixed names
where that is appropriate for name space issues--__foo being wholly
identical to foo, not a wrapper doing something slightly different.
It's a bonus that it makes it less likely you'll encounter
mutually-dependent #include hell, but for me the chief motivation is
ease of understanding the code: If a source file does #include <foo.h>
for a standard <foo.h> and then uses standard foo.h APIs (or ones that
look just like them except for something trivial-seeming like the __
prefix), then the reader who knows the standards but not every corner of
our internal headers should not be misled about what functions are being
called and what their exact APIs are.

Once we have a clean and happy situation for a particular interface in
our internal code, then we can carefully consider the potential utility
of offering a new public API that brings similar benefits to application
code.  But I don't think we should be eager to try to find such things,
while we certainly should be eager to do internal cleanups.


Thanks,
Roland
Alexandre Oliva Dec. 16, 2014, 7:03 p.m. UTC | #5
I'd like to single out one point from the previous discussion to
elaborate on and attempt to reach consensus on policy regarding
poisoning symbols to enforce best practice:

On Nov  5, 2014, Alexandre Oliva <aoliva@redhat.com> wrote:

> There are various ways to go about fixing this, ranging from using
> __poll and __socket internal wrappers/aliases to avoid the undesirable
> problems in the first place (say, having __poll never return EINTR, but
> retrying with a recomputed timeout instead instead, and having __socket
> always set CLOEXEC), introducing alternate functions to do so and
> perhaps poisoning the original symbols so that misuses are flagged, up
> to just offering wrappers that implement the best practice in internal
> header wrappers or new internal headers.  AFAICT we don't seem to have
> any recommended practice on how to factor out, let alone internally
> enforce, this sort of best practice.

Having socket always set CLOEXEC turned out to be a bad example, because
in some cases we want to create sockets that do not have CLOEXEC set.

This might seem like an argument against poisoning the socket symbols to
avoid their internal use in glibc, but maybe we'd be better off changing
them to *another* wrapper that indicates whether the socket they create
is internal or not through e.g. passing SOCK_CLOEXEC in the flags, so
that the wrapper can fallback to __socket_cloexec if the flag is set.

IIRC there are only uses in sunrpc that might have benefitted from this,
but if we wanted to enforce an explicit decision on whether or not we
want SOCK_CLOEXEC at each socket creation point, poisoning the __socket
and socket tokens, and introducing such wrappers may be a good way to
implement it.

The ultimate goal is that, at some point, no explicit socket calls would
remain, and at that point we will know that for every socket call the
decision on whether or not to use SOCK_CLOEXEC was thought about and
made, rather than the current state, in which there are plenty of socket
calls that each require some (occasionally non-trivial) analysis to
figure out whether or not they should have the SOCK_CLOEXEC flag set.
We don't want to have to repeat such analysis over and over and over.

Of course, just adding comments is a way to avoid costly analysis, but
in general the comments won't be at the same line as the call, so at
least some time will be wasted after a global grep for unadorned calls.
Using wrappers that indicate the analysis was done before avoids that.


Now, another issue is that so far the proposed wrappers have been in
separate headers, so that their use (and the symbol poisoning that might
come with them) has been voluntary.

In order to enforce best practice and informed decisions all over glibc,
this is not enough.  We'd have to have the wrappers in the internal
headers that are already used all over to get a declaration for the
wrapped functions in the first place.  Without that, working around
policy becomes as simple as refraining from opting into the
policy-enforcing headers.


So here's what I propose:

Introduce internal header include/poll-noeintr.h, with the
__poll_noeintr inline wrapper function that retries if interrupted by
signals, and #pragma GCC poison poll __poll, so that all uses of poll
and __poll will be flagged and replaced with __poll_noeintr.  Then,
include poll-noeintr.h at the end of sys/poll.h, so that the use of
poll_noeintr is enforced all over.

Introduce internal header include/socket-cloexec.h, with
__socket_cloexec and __socket_xflag inline wrapper functions, so that we
can poison socket and __socket, so that all uses of the poisoned symbols
have to call either __socket_cloexec, for sockets that are definitely
internal to libc and that therefore should ideally have the CLOEXEC flag
set, or __socket_xflag, that explicitly flags whether the socket is
internal or external by passing it the SOCK_CLOEXEC flag.  Then, include
socket-cloexec.h in sys/socket.h, so that uses of __socket that have not
been replaced with either __socket_cloexec or __socket_xflag are flagged
and fixed.

In general, whenever we want to enforce internally one particular use
pattern for a function, introduce a header that introduces one or more
wrappers for that function, covering all sorts of internal uses and
poisoning the function's original symbols.  Then, include this header at
the end of the internal header that declares the original symbols, so
that remaining uses are flagged and the policy is enforced all over.

Patch
diff mbox

create all sockets with SOCK_CLOEXEC

From: Alexandre Oliva <aoliva@redhat.com>


---
 include/socket-cloexec.h               |  105 ++++++++++++++++++++++++++++++++
 resolv/res_hconf.c                     |    3 +
 socket/opensock.c                      |   15 ++---
 sunrpc/clnt_tcp.c                      |    3 +
 sunrpc/clnt_unix.c                     |    3 +
 sunrpc/pm_getport.c                    |    3 +
 sunrpc/pmap_rmt.c                      |    3 +
 sunrpc/rtime.c                         |    3 +
 sunrpc/svc_tcp.c                       |    4 +
 sunrpc/svc_udp.c                       |    4 +
 sunrpc/svc_unix.c                      |    3 +
 sysdeps/gnu/ifaddrs.c                  |    3 +
 sysdeps/posix/getaddrinfo.c            |    3 +
 sysdeps/unix/sysv/linux/check_native.c |    4 +
 sysdeps/unix/sysv/linux/check_pf.c     |    3 +
 sysdeps/unix/sysv/linux/ifaddrs.c      |    3 +
 16 files changed, 144 insertions(+), 21 deletions(-)
 create mode 100644 include/socket-cloexec.h

diff --git a/include/socket-cloexec.h b/include/socket-cloexec.h
new file mode 100644
index 0000000..8ada604
--- /dev/null
+++ b/include/socket-cloexec.h
@@ -0,0 +1,105 @@ 
+/* Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _SOCKET_CLOEXEC_H
+#define _SOCKET_CLOEXEC_H
+
+#include <stdbool.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/socket.h>
+
+/* Like socket, but with SOCK_CLOEXEC set if available.  If it's not,
+   try to set the FD_CLOEXEC flag, and if that fails, close the socket
+   and fail unless __tolerant.  */
+static inline int
+__socket_cloexec (int __domain, int __type, int __protocol, bool __tolerant)
+{
+  int __ret;
+#ifdef SOCK_CLOEXEC
+# ifdef __ASSUME_SOCK_CLOEXEC
+  int  __have_sock_cloexec = 1;
+# endif /* __ASSUME_SOCK_CLOEXEC */
+  if (__have_sock_cloexec >= 0)
+    {
+      __ret = __socket (__domain, __type | SOCK_CLOEXEC, __protocol);
+      if (__have_sock_cloexec == 0)
+	__have_sock_cloexec = (__ret == -1 && errno == EINVAL ? -1 : 1);
+      if (__have_sock_cloexec > 0)
+	return __ret;
+    }
+#endif /* SOCK_CLOEXEC */
+  __ret = __socket (__domain, __type, __protocol);
+
+  if ((__ret >= 0 && __fcntl (__ret, F_SETFD, FD_CLOEXEC) < 0) && !__tolerant)
+    {
+      __close (__ret);
+      __ret = -1;
+    }
+
+  return __ret;
+}
+
+/* Like socket, but with SOCK_CLOEXEC and SOCK_NONBLOCK set if
+   SOCK_CLOEXEC is available.  SOCK_NONBLOCK is supported iff
+   SOCK_CLOEXEC is.
+
+   If SOCK_CLOEXEC is not supported, try to set the FD_CLOEXEC flag,
+   and if that fails, close the socket and fail unless __tolerant.
+
+   If SOCK_NONBLOCK is not supported, try to set the O_NONBLOCK flag,
+   and if that fails, close the socket and fail REGARDLESS of
+   __tolerant.  */
+static inline int
+__socket_cloexec_nonblock (int __domain, int __type, int __protocol,
+			   bool __tolerant)
+{
+  int __ret;
+#ifdef SOCK_NONBLOCK
+# ifdef __ASSUME_SOCK_CLOEXEC
+  int  __have_sock_cloexec = 1;
+# endif /* __ASSUME_SOCK_CLOEXEC */
+  if (__have_sock_cloexec >= 0)
+    {
+      __ret = __socket (__domain, __type | SOCK_CLOEXEC | SOCK_NONBLOCK,
+			__protocol);
+      if (__have_sock_cloexec == 0)
+	__have_sock_cloexec = (__ret == -1 && errno == EINVAL ? -1 : 1);
+      if (__have_sock_cloexec > 0)
+	return __ret;
+    }
+#endif /* SOCK_NONBLOCK */
+  __ret = __socket (__domain, __type, __protocol);
+  if (__ret >= 0)
+    {
+      int __fl = __fcntl (__ret, F_GETFL);
+      if (__fl == -1
+	  || __fcntl (__ret, F_SETFL, __fl | O_NONBLOCK) < 0
+	  || (__fcntl (__ret, F_SETFD, FD_CLOEXEC) < 0 && !__tolerant))
+	{
+	  __close (__ret);
+	  __ret = -1;
+	}
+    }
+  return __ret;
+}
+
+#pragma poison __socket
+#pragma poison socket
+
+#endif /* _SOCKET_CLOEXEC_H */
diff --git a/resolv/res_hconf.c b/resolv/res_hconf.c
index b4c86227..1df336c 100644
--- a/resolv/res_hconf.c
+++ b/resolv/res_hconf.c
@@ -41,6 +41,7 @@ 
 #include <sys/ioctl.h>
 #include <unistd.h>
 #include <netinet/in.h>
+#include <socket-cloexec.h>
 #include <bits/libc-lock.h>
 #include "ifreq.h"
 #include "res_hconf.h"
@@ -410,7 +411,7 @@  _res_hconf_reorder_addrs (struct hostent *hp)
       /* Initialize interface table.  */
 
       /* The SIOCGIFNETMASK ioctl will only work on an AF_INET socket.  */
-      sd = __socket (AF_INET, SOCK_DGRAM, 0);
+      sd = __socket_cloexec (AF_INET, SOCK_DGRAM, 0, true);
       if (sd < 0)
 	return;
 
diff --git a/socket/opensock.c b/socket/opensock.c
index 8dd8906..d23a962 100644
--- a/socket/opensock.c
+++ b/socket/opensock.c
@@ -17,6 +17,7 @@ 
 
 #include <stdio.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <bits/libc-lock.h>
 
 /* Return a socket of any type.  The socket can be used in subsequent
@@ -32,7 +33,7 @@  __opensock (void)
 
   if (sock_af != -1)
     {
-      fd = __socket (sock_af, SOCK_DGRAM, 0);
+      fd = __socket_cloexec (sock_af, SOCK_DGRAM, 0, true);
       if (fd != -1)
         return fd;
     }
@@ -40,28 +41,28 @@  __opensock (void)
   __libc_lock_lock (lock);
 
   if (sock_af != -1)
-    fd = __socket (sock_af, SOCK_DGRAM, 0);
+    fd = __socket_cloexec (sock_af, SOCK_DGRAM, 0, true);
 
   if (fd == -1)
     {
 #ifdef AF_INET
-      fd = __socket (sock_af = AF_INET, SOCK_DGRAM, 0);
+      fd = __socket_cloexec (sock_af = AF_INET, SOCK_DGRAM, 0, true);
 #endif
 #ifdef AF_INET6
       if (fd < 0)
-	fd = __socket (sock_af = AF_INET6, SOCK_DGRAM, 0);
+	fd = __socket_cloexec (sock_af = AF_INET6, SOCK_DGRAM, 0, true);
 #endif
 #ifdef AF_IPX
       if (fd < 0)
-	fd = __socket (sock_af = AF_IPX, SOCK_DGRAM, 0);
+	fd = __socket_cloexec (sock_af = AF_IPX, SOCK_DGRAM, 0, true);
 #endif
 #ifdef AF_AX25
       if (fd < 0)
-	fd = __socket (sock_af = AF_AX25, SOCK_DGRAM, 0);
+	fd = __socket_cloexec (sock_af = AF_AX25, SOCK_DGRAM, 0, true);
 #endif
 #ifdef AF_APPLETALK
       if (fd < 0)
-	fd = __socket (sock_af = AF_APPLETALK, SOCK_DGRAM, 0);
+	fd = __socket_cloexec (sock_af = AF_APPLETALK, SOCK_DGRAM, 0, true);
 #endif
     }
 
diff --git a/sunrpc/clnt_tcp.c b/sunrpc/clnt_tcp.c
index f4ba0ef..2dd07cf 100644
--- a/sunrpc/clnt_tcp.c
+++ b/sunrpc/clnt_tcp.c
@@ -52,6 +52,7 @@ 
 #include <rpc/rpc.h>
 #include <sys/poll.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <rpc/pmap_clnt.h>
 #include <wchar.h>
 
@@ -146,7 +147,7 @@  clnttcp_create (struct sockaddr_in *raddr, u_long prog, u_long vers,
    */
   if (*sockp < 0)
     {
-      *sockp = __socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+      *sockp = __socket_cloexec (AF_INET, SOCK_STREAM, IPPROTO_TCP, true);
       (void) bindresvport (*sockp, (struct sockaddr_in *) 0);
       if ((*sockp < 0)
 	  || (__connect (*sockp, (struct sockaddr *) raddr,
diff --git a/sunrpc/clnt_unix.c b/sunrpc/clnt_unix.c
index aff6fa5..419a58a 100644
--- a/sunrpc/clnt_unix.c
+++ b/sunrpc/clnt_unix.c
@@ -53,6 +53,7 @@ 
 #include <sys/uio.h>
 #include <sys/poll.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <rpc/pmap_clnt.h>
 #include <wchar.h>
 
@@ -132,7 +133,7 @@  clntunix_create (struct sockaddr_un *raddr, u_long prog, u_long vers,
    */
   if (*sockp < 0)
     {
-      *sockp = __socket (AF_UNIX, SOCK_STREAM, 0);
+      *sockp = __socket_cloexec (AF_UNIX, SOCK_STREAM, 0, true);
       len = strlen (raddr->sun_path) + sizeof (raddr->sun_family) + 1;
       if (*sockp < 0
 	  || __connect (*sockp, (struct sockaddr *) raddr, len) < 0)
diff --git a/sunrpc/pm_getport.c b/sunrpc/pm_getport.c
index d9df0cc..31f8dcc 100644
--- a/sunrpc/pm_getport.c
+++ b/sunrpc/pm_getport.c
@@ -38,6 +38,7 @@ 
 #include <rpc/pmap_prot.h>
 #include <rpc/pmap_clnt.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 
 /*
  * Create a socket that is locally bound to a non-reserve port. For
@@ -48,7 +49,7 @@  int
 internal_function
 __get_socket (struct sockaddr_in *saddr)
 {
-  int so = __socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+  int so = __socket_cloexec (AF_INET, SOCK_STREAM, IPPROTO_TCP, true);
   if (so < 0)
     return -1;
 
diff --git a/sunrpc/pmap_rmt.c b/sunrpc/pmap_rmt.c
index fd8de85..389eb9e 100644
--- a/sunrpc/pmap_rmt.c
+++ b/sunrpc/pmap_rmt.c
@@ -42,6 +42,7 @@ 
 #include <rpc/pmap_rmt.h>
 #include <sys/poll.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <stdio.h>
 #include <errno.h>
 #undef	 _POSIX_SOURCE		/* Ultrix <sys/param.h> needs --roland@gnu */
@@ -238,7 +239,7 @@  clnt_broadcast (prog, vers, proc, xargs, argsp, xresults, resultsp, eachresult)
    * initialization: create a socket, a broadcast address, and
    * preserialize the arguments into a send buffer.
    */
-  if ((sock = __socket (AF_INET, SOCK_DGRAM, IPPROTO_UDP)) < 0)
+  if ((sock = __socket_cloexec (AF_INET, SOCK_DGRAM, IPPROTO_UDP, true)) < 0)
     {
       perror (_("Cannot create socket for broadcast rpc"));
       stat = RPC_CANTSEND;
diff --git a/sunrpc/rtime.c b/sunrpc/rtime.c
index 79d55d1..48f4681 100644
--- a/sunrpc/rtime.c
+++ b/sunrpc/rtime.c
@@ -45,6 +45,7 @@ 
 #include <sys/types.h>
 #include <sys/poll.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <sys/time.h>
 #include <rpc/auth_des.h>
 #include <errno.h>
@@ -84,7 +85,7 @@  rtime (struct sockaddr_in *addrp, struct rpc_timeval *timep,
   else
     type = SOCK_DGRAM;
 
-  s = __socket (AF_INET, type, 0);
+  s = __socket_cloexec (AF_INET, type, 0, true);
   if (s < 0)
     return (-1);
 
diff --git a/sunrpc/svc_tcp.c b/sunrpc/svc_tcp.c
index 92886f0..eaf6297 100644
--- a/sunrpc/svc_tcp.c
+++ b/sunrpc/svc_tcp.c
@@ -58,6 +58,7 @@ 
 #include <libintl.h>
 #include <rpc/rpc.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <sys/poll.h>
 #include <errno.h>
 #include <stdlib.h>
@@ -159,7 +160,8 @@  svctcp_create (int sock, u_int sendsize, u_int recvsize)
 
   if (sock == RPC_ANYSOCK)
     {
-      if ((sock = __socket (AF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0)
+      if ((sock = __socket_cloexec (AF_INET, SOCK_STREAM, IPPROTO_TCP,
+				    true)) < 0)
 	{
 	  perror (_("svc_tcp.c - tcp socket creation problem"));
 	  return (SVCXPRT *) NULL;
diff --git a/sunrpc/svc_udp.c b/sunrpc/svc_udp.c
index 411234a..f7a6da1 100644
--- a/sunrpc/svc_udp.c
+++ b/sunrpc/svc_udp.c
@@ -55,6 +55,7 @@ 
 #include <string.h>
 #include <rpc/rpc.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <errno.h>
 #include <libintl.h>
 
@@ -132,7 +133,8 @@  svcudp_bufcreate (sock, sendsz, recvsz)
 
   if (sock == RPC_ANYSOCK)
     {
-      if ((sock = __socket (AF_INET, SOCK_DGRAM, IPPROTO_UDP)) < 0)
+      if ((sock = __socket_cloexec (AF_INET, SOCK_DGRAM, IPPROTO_UDP,
+				    true)) < 0)
 	{
 	  perror (_("svcudp_create: socket creation problem"));
 	  return (SVCXPRT *) NULL;
diff --git a/sunrpc/svc_unix.c b/sunrpc/svc_unix.c
index 6d93c5f..0d7466d 100644
--- a/sunrpc/svc_unix.c
+++ b/sunrpc/svc_unix.c
@@ -58,6 +58,7 @@ 
 #include <rpc/rpc.h>
 #include <rpc/svc.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <sys/uio.h>
 #include <sys/poll.h>
 #include <errno.h>
@@ -157,7 +158,7 @@  svcunix_create (int sock, u_int sendsize, u_int recvsize, char *path)
 
   if (sock == RPC_ANYSOCK)
     {
-      if ((sock = __socket (AF_UNIX, SOCK_STREAM, 0)) < 0)
+      if ((sock = __socket_cloexec (AF_UNIX, SOCK_STREAM, 0, true)) < 0)
 	{
 	  perror (_("svc_unix.c - AF_UNIX socket creation problem"));
 	  return (SVCXPRT *) NULL;
diff --git a/sysdeps/gnu/ifaddrs.c b/sysdeps/gnu/ifaddrs.c
index 1b8775f..70db461 100644
--- a/sysdeps/gnu/ifaddrs.c
+++ b/sysdeps/gnu/ifaddrs.c
@@ -19,6 +19,7 @@ 
 #include <ifaddrs.h>
 #include <net/if.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <sys/ioctl.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -37,7 +38,7 @@  getifaddrs (struct ifaddrs **ifap)
   /* This implementation handles only IPv4 interfaces.
      The various ioctls below will only work on an AF_INET socket.
      Some different mechanism entirely must be used for IPv6.  */
-  int fd = __socket (AF_INET, SOCK_DGRAM, 0);
+  int fd = __socket_cloexec (AF_INET, SOCK_DGRAM, 0, true);
   struct ifreq *ifreqs;
   int nifs;
 
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 8f392b9..3cbd033 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -52,6 +52,7 @@  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <net/if.h>
 #include <netinet/in.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/un.h>
@@ -2512,7 +2513,7 @@  getaddrinfo (const char *name, const char *service,
 		  close_retry:
 		    close_not_cancel_no_status (fd);
 		  af = q->ai_family;
-		  fd = __socket (af, SOCK_DGRAM, IPPROTO_IP);
+		  fd = __socket_cloexec (af, SOCK_DGRAM, IPPROTO_IP, true);
 		}
 	      else
 		{
diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
index 68adeb8..4a2e143 100644
--- a/sysdeps/unix/sysv/linux/check_native.c
+++ b/sysdeps/unix/sysv/linux/check_native.c
@@ -28,6 +28,8 @@ 
 #include <net/if.h>
 #include <net/if_arp.h>
 #include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <socket-cloexec.h>
 
 #include <asm/types.h>
 #include <linux/netlink.h>
@@ -40,7 +42,7 @@  void
 __check_native (uint32_t a1_index, int *a1_native,
 		uint32_t a2_index, int *a2_native)
 {
-  int fd = __socket (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+  int fd = __socket_cloexec (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE, true);
 
   struct sockaddr_nl nladdr;
   memset (&nladdr, '\0', sizeof (nladdr));
diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/linux/check_pf.c
index 976f249e..a098068 100644
--- a/sysdeps/unix/sysv/linux/check_pf.c
+++ b/sysdeps/unix/sysv/linux/check_pf.c
@@ -26,6 +26,7 @@ 
 #include <unistd.h>
 #include <stdint.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 
 #include <asm/types.h>
 #include <linux/netlink.h>
@@ -353,7 +354,7 @@  __check_pf (bool *seen_ipv4, bool *seen_ipv6,
     }
   else
     {
-      int fd = __socket (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+      int fd = __socket_cloexec (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE, true);
 
       if (__glibc_likely (fd >= 0))
 	{
diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c
index a47b2ed..aed494e 100644
--- a/sysdeps/unix/sysv/linux/ifaddrs.c
+++ b/sysdeps/unix/sysv/linux/ifaddrs.c
@@ -29,6 +29,7 @@ 
 #include <string.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
+#include <socket-cloexec.h>
 #include <sysdep.h>
 #include <time.h>
 #include <unistd.h>
@@ -251,7 +252,7 @@  __netlink_open (struct netlink_handle *h)
 {
   struct sockaddr_nl nladdr;
 
-  h->fd = __socket (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+  h->fd = __socket_cloexec (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE, true);
   if (h->fd < 0)
     goto out;