[PATCH] linux/check_native: Always close socket on return

[PATCH] linux/check_native: Always close socket on return

[Siddhesh Poyarekar]

The error paths of __check_native would leave the socket FD open on
return, resulting in an FD leak.  Rework function exit paths so that
the fd is always closed on return.
---
 sysdeps/unix/sysv/linux/check_native.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
index e4e6e80dbc..47b339b629 100644
--- a/sysdeps/unix/sysv/linux/check_native.c
+++ b/sysdeps/unix/sysv/linux/check_native.c
@@ -49,11 +49,13 @@ __check_native (uint32_t a1_index, int *a1_native,
 
   socklen_t addr_len = sizeof (nladdr);
 
-  if (fd < 0
-      || __bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
-      || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
+  if (fd < 0)
     return;
 
+  if (__bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
+      || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
+    goto out;
+
   pid_t pid = nladdr.nl_pid;
   struct req
   {
@@ -96,7 +98,7 @@ __check_native (uint32_t a1_index, int *a1_native,
       if (buf != NULL)
 	use_malloc = true;
       else
-	goto out_fail;
+	goto out;
     }
 
   struct iovec iov = { buf, buf_size };
@@ -104,7 +106,7 @@ __check_native (uint32_t a1_index, int *a1_native,
   if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
 				    (struct sockaddr *) &nladdr,
 				    sizeof (nladdr))) < 0)
-    goto out_fail;
+    goto out;
 
   bool done = false;
   do
@@ -123,10 +125,10 @@ __check_native (uint32_t a1_index, int *a1_native,
       ssize_t read_len = TEMP_FAILURE_RETRY (__recvmsg (fd, &msg, 0));
       __netlink_assert_response (fd, read_len);
       if (read_len < 0)
-	goto out_fail;
+	goto out;
 
       if (msg.msg_flags & MSG_TRUNC)
-	goto out_fail;
+	goto out;
 
       struct nlmsghdr *nlmh;
       for (nlmh = (struct nlmsghdr *) buf;
@@ -166,12 +168,9 @@ __check_native (uint32_t a1_index, int *a1_native,
     }
   while (! done);
 
- out:
+out:
   __close_nocancel_nostatus (fd);
 
-  return;
-
-out_fail:
   if (use_malloc)
     free (buf);
 }
-- 
2.31.1

Re: [PATCH] linux/check_native: Always close socket on return

[Andreas Schwab]

On Mai 12 2021, Siddhesh Poyarekar via Libc-alpha wrote:

> @@ -166,12 +168,9 @@ __check_native (uint32_t a1_index, int *a1_native,
>      }
>    while (! done);
>  
> - out:
> +out:
>    __close_nocancel_nostatus (fd);
>  
> -  return;
> -
> -out_fail:
>    if (use_malloc)

use_malloc is used uninitialized.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH v2] linux/check_native: Always close socket on return

[Siddhesh Poyarekar]

The error paths of __check_native would leave the socket FD open on
return, resulting in an FD leak.  Rework function exit paths so that
the fd is always closed on return.
---
 sysdeps/unix/sysv/linux/check_native.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
index e4e6e80dbc..13e5eaf339 100644
--- a/sysdeps/unix/sysv/linux/check_native.c
+++ b/sysdeps/unix/sysv/linux/check_native.c
@@ -48,12 +48,15 @@ __check_native (uint32_t a1_index, int *a1_native,
   nladdr.nl_family = AF_NETLINK;
 
   socklen_t addr_len = sizeof (nladdr);
+  bool use_malloc = false;
 
-  if (fd < 0
-      || __bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
-      || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
+  if (fd < 0)
     return;
 
+  if (__bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
+      || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
+    goto out;
+
   pid_t pid = nladdr.nl_pid;
   struct req
   {
@@ -85,7 +88,6 @@ __check_native (uint32_t a1_index, int *a1_native,
 #else
   const size_t buf_size = __getpagesize ();
 #endif
-  bool use_malloc = false;
   char *buf;
 
   if (__libc_use_alloca (buf_size))
@@ -96,7 +98,7 @@ __check_native (uint32_t a1_index, int *a1_native,
       if (buf != NULL)
 	use_malloc = true;
       else
-	goto out_fail;
+	goto out;
     }
 
   struct iovec iov = { buf, buf_size };
@@ -104,7 +106,7 @@ __check_native (uint32_t a1_index, int *a1_native,
   if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
 				    (struct sockaddr *) &nladdr,
 				    sizeof (nladdr))) < 0)
-    goto out_fail;
+    goto out;
 
   bool done = false;
   do
@@ -123,10 +125,10 @@ __check_native (uint32_t a1_index, int *a1_native,
       ssize_t read_len = TEMP_FAILURE_RETRY (__recvmsg (fd, &msg, 0));
       __netlink_assert_response (fd, read_len);
       if (read_len < 0)
-	goto out_fail;
+	goto out;
 
       if (msg.msg_flags & MSG_TRUNC)
-	goto out_fail;
+	goto out;
 
       struct nlmsghdr *nlmh;
       for (nlmh = (struct nlmsghdr *) buf;
@@ -166,12 +168,9 @@ __check_native (uint32_t a1_index, int *a1_native,
     }
   while (! done);
 
- out:
+out:
   __close_nocancel_nostatus (fd);
 
-  return;
-
-out_fail:
   if (use_malloc)
     free (buf);
 }
-- 
2.31.1

Re: [PATCH v2] linux/check_native: Always close socket on return

[Andreas Schwab]

On Mai 12 2021, Siddhesh Poyarekar wrote:

> The error paths of __check_native would leave the socket FD open on
> return, resulting in an FD leak.  Rework function exit paths so that
> the fd is always closed on return.

Ok.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."