]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
nss_files: Fix re-reading of long lines [BZ #18991]
authorFlorian Weimer <fweimer@redhat.com>
Fri, 6 Jul 2018 12:23:15 +0000 (14:23 +0200)
committerFlorian Weimer <fweimer@redhat.com>
Fri, 6 Jul 2018 15:52:54 +0000 (17:52 +0200)
Use the new __libc_readline_unlocked function to pick up
reading at the same line in case the buffer needs to be enlarged.

ChangeLog
nss/Makefile
nss/nss_files/files-XXX.c
nss/tst-nss-files-hosts-getent.c [new file with mode: 0644]

index 51e6bdcefc5efeb34efb3f221513622cd4a92475..7a052c344a10cf3c9c89bf8ad1c8e5a535cab192 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2018-07-06  Florian Weimer  <fweimer@redhat.com>
+
+       [BZ #18991]
+       * nss/nss_files/files-XXX.c (internal_getent): Use
+       __libc_readline_unlocked.  Seek back to the start of the line if
+       parsing failes with ERANGE.
+       (get_contents_ret, get_contents): Remove.
+       * nss/tst-nss-files-hosts-getent.c: New file.
+       * nss/Makefile (tests): Add tst-nss-files-hosts-getent.
+       (tst-nss-files-hosts-getent): Link with -ldl.
+
 2018-07-06  Florian Weimer  <fweimer@redhat.com>
 
        * include/stdio.h (__libc_readline_unlocked): Declare.
index a5cd2aacae8f7813461f1998d4508b559278e2b1..66fac7f5b8a4c0d84f8afcc0be302e57c0520a6f 100644 (file)
@@ -64,6 +64,7 @@ xtests                        = bug-erange
 ifeq (yes,$(build-shared))
 tests += tst-nss-files-hosts-erange
 tests += tst-nss-files-hosts-multi
+tests += tst-nss-files-hosts-getent
 endif
 
 # If we have a thread library then we can test cancellation against
@@ -169,3 +170,4 @@ endif
 
 $(objpfx)tst-nss-files-hosts-erange: $(libdl)
 $(objpfx)tst-nss-files-hosts-multi: $(libdl)
+$(objpfx)tst-nss-files-hosts-getent: $(libdl)
index df33551258797f556e1d68d2e29c147d71fae943..60d1544a96b12f7ad019bdeb6b07207e68c67be8 100644 (file)
@@ -128,51 +128,6 @@ CONCAT(_nss_files_end,ENTNAME) (void)
 }
 \f
 
-typedef enum
-{
-  gcr_ok = 0,
-  gcr_error = -1,
-  gcr_overflow = -2
-} get_contents_ret;
-
-/* Hack around the fact that fgets only accepts int sizes.  */
-static get_contents_ret
-get_contents (char *linebuf, size_t len, FILE *stream)
-{
-  size_t remaining_len = len;
-  char *curbuf = linebuf;
-
-  do
-    {
-      int curlen = ((remaining_len > (size_t) INT_MAX) ? INT_MAX
-                   : remaining_len);
-
-      /* Terminate the line so that we can test for overflow.  */
-      ((unsigned char *) curbuf)[curlen - 1] = 0xff;
-
-      char *p = fgets_unlocked (curbuf, curlen, stream);
-
-      /* EOF or read error.  */
-      if (p == NULL)
-        return gcr_error;
-
-      /* Done reading in the line.  */
-      if (((unsigned char *) curbuf)[curlen - 1] == 0xff)
-        return gcr_ok;
-
-      /* Drop the terminating '\0'.  */
-      remaining_len -= curlen - 1;
-      curbuf += curlen - 1;
-    }
-  /* fgets copies one less than the input length.  Our last iteration is of
-     REMAINING_LEN and once that is done, REMAINING_LEN is decremented by
-     REMAINING_LEN - 1, leaving the result as 1.  */
-  while (remaining_len > 1);
-
-  /* This means that the current buffer was not large enough.  */
-  return gcr_overflow;
-}
-
 /* Parsing the database file into `struct STRUCTURE' data structures.  */
 static enum nss_status
 internal_getent (FILE *stream, struct STRUCTURE *result,
@@ -191,45 +146,69 @@ internal_getent (FILE *stream, struct STRUCTURE *result,
       return NSS_STATUS_TRYAGAIN;
     }
 
-  do
+  while (true)
     {
-      get_contents_ret r = get_contents (data->linebuffer, linebuflen, stream);
-
-      if (r == gcr_error)
+      ssize_t r = __libc_readline_unlocked
+       (stream, data->linebuffer, linebuflen);
+      if (r < 0)
+       {
+         *errnop = errno;
+         H_ERRNO_SET (NETDB_INTERNAL);
+         if (*errnop == ERANGE)
+           /* Request larger buffer.  */
+           return NSS_STATUS_TRYAGAIN;
+         else
+           /* Other read failure.  */
+           return NSS_STATUS_UNAVAIL;
+       }
+      else if (r == 0)
        {
-         /* End of file or read error.  */
+         /* End of file.  */
          H_ERRNO_SET (HOST_NOT_FOUND);
          return NSS_STATUS_NOTFOUND;
        }
 
-      if (r == gcr_overflow)
+      /* Everything OK.  Now skip leading blanks.  */
+      p = data->linebuffer;
+      while (isspace (*p))
+       ++p;
+
+      /* Ignore empty and comment lines.  */
+      if (*p == '\0' || *p == '#')
+       continue;
+
+      /* Parse the line.   */
+      *errnop = EINVAL;
+      parse_result = parse_line (p, result, data, buflen, errnop EXTRA_ARGS);
+
+      if (parse_result == -1)
        {
-         /* The line is too long.  Give the user the opportunity to
-            enlarge the buffer.  */
-         *errnop = ERANGE;
+         if (*errnop == ERANGE)
+           {
+             /* Return to the original file position at the beginning
+                of the line, so that the next call can read it again
+                if necessary.  */
+             if (__fseeko64 (stream, -r, SEEK_CUR) != 0)
+               {
+                 if (errno == ERANGE)
+                   *errnop = EINVAL;
+                 else
+                   *errnop = errno;
+                 H_ERRNO_SET (NETDB_INTERNAL);
+                 return NSS_STATUS_UNAVAIL;
+               }
+           }
          H_ERRNO_SET (NETDB_INTERNAL);
          return NSS_STATUS_TRYAGAIN;
        }
 
-      /* Everything OK.  Now skip leading blanks.  */
-      p = data->linebuffer;
-      while (isspace (*p))
-       ++p;
-    }
-  while (*p == '\0' || *p == '#' /* Ignore empty and comment lines.  */
-        /* Parse the line.  If it is invalid, loop to get the next
-           line of the file to parse.  */
-        || ! (parse_result = parse_line (p, result, data, buflen, errnop
-                                         EXTRA_ARGS)));
+      /* Return the data if parsed successfully.  */
+      if (parse_result != 0)
+       return NSS_STATUS_SUCCESS;
 
-  if (__glibc_unlikely (parse_result == -1))
-    {
-      H_ERRNO_SET (NETDB_INTERNAL);
-      return NSS_STATUS_TRYAGAIN;
+      /* If it is invalid, loop to get the next line of the file to
+        parse.  */
     }
-
-  /* Filled in RESULT with the next entry from the database file.  */
-  return NSS_STATUS_SUCCESS;
 }
 
 
diff --git a/nss/tst-nss-files-hosts-getent.c b/nss/tst-nss-files-hosts-getent.c
new file mode 100644 (file)
index 0000000..d7514e8
--- /dev/null
@@ -0,0 +1,276 @@
+/* Enumerate /etc/hosts with a long line (bug 18991).
+   Copyright (C) 2017 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/>.  */
+
+
+#include <dlfcn.h>
+#include <errno.h>
+#include <gnu/lib-names.h>
+#include <netdb.h>
+#include <nss.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/check_nss.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+#include <support/xmemstream.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+struct support_chroot *chroot_env;
+
+/* Number of alias names in the long line.  This is varied to catch
+   different cases where the ERANGE handling can go wrong (line buffer
+   length, alias buffer).  */
+static int name_count;
+
+/* Write /etc/hosts, from outside of the chroot.  */
+static void
+write_hosts (void)
+{
+  FILE *fp = xfopen (chroot_env->path_hosts, "w");
+  fputs ("127.0.0.1   localhost localhost.localdomain\n", fp);
+  fputs ("192.0.2.2 host2.example.com\n", fp);
+  fputs ("192.0.2.1", fp);
+  for (int i = 0; i < name_count; ++i)
+    fprintf (fp, " host%d.example.com", i);
+  fputs ("\n192.0.2.80 www.example.com\n"
+         "192.0.2.5 host5.example.com\n"
+         "192.0.2.81 www1.example.com\n", fp);
+  xfclose (fp);
+}
+
+const char *host1_expected =
+  "name: localhost\n"
+  "alias: localhost.localdomain\n"
+  "address: 127.0.0.1\n";
+const char *host2_expected =
+  "name: host2.example.com\n"
+  "address: 192.0.2.2\n";
+const char *host4_expected =
+  "name: www.example.com\n"
+  "address: 192.0.2.80\n";
+const char *host5_expected =
+  "name: host5.example.com\n"
+  "address: 192.0.2.5\n";
+const char *host6_expected =
+  "name: www1.example.com\n"
+  "address: 192.0.2.81\n";
+
+static void
+prepare (int argc, char **argv)
+{
+  chroot_env = support_chroot_create
+    ((struct support_chroot_configuration)
+     {
+       .resolv_conf = "",
+       .hosts = "",             /* Filled in by write_hosts.  */
+       .host_conf = "multi on\n",
+     });
+}
+
+/* If -1, no sethostent call.  Otherwise, pass do_stayopen as the
+   sethostent argument.  */
+static int do_stayopen;
+
+/* If non-zero, perform an endostent call.  */
+static int do_endent;
+
+static void
+subprocess_getent (void *closure)
+{
+  xchroot (chroot_env->path_chroot);
+
+  errno = 0;
+  if (do_stayopen >= 0)
+    sethostent (do_stayopen);
+  TEST_VERIFY (errno == 0);
+
+  int i = 0;
+  while (true)
+    {
+      struct xmemstream expected;
+      xopen_memstream (&expected);
+      switch (++i)
+        {
+        case 1:
+          fputs (host1_expected, expected.out);
+          break;
+        case 2:
+          fputs (host2_expected, expected.out);
+          break;
+        case 3:
+          fputs ("name: host0.example.com\n", expected.out);
+          for (int j = 1; j < name_count; ++j)
+            fprintf (expected.out, "alias: host%d.example.com\n", j);
+          fputs ("address: 192.0.2.1\n", expected.out);
+          break;
+        case 4:
+          fputs (host4_expected, expected.out);
+          break;
+        case 5:
+          fputs (host5_expected, expected.out);
+          break;
+        case 6:
+          fputs (host6_expected, expected.out);
+          break;
+        default:
+          fprintf (expected.out, "*** unexpected host %d ***\n", i);
+          break;
+        }
+      xfclose_memstream (&expected);
+      char *context = xasprintf ("do_stayopen=%d host=%d", do_stayopen, i);
+
+      errno = 0;
+      struct hostent *e = gethostent ();
+      if (e == NULL)
+        {
+          TEST_VERIFY (errno == 0);
+          break;
+        }
+      check_hostent (context, e, expected.buffer);
+      free (context);
+      free (expected.buffer);
+    }
+
+  errno = 0;
+  if (do_endent)
+    endhostent ();
+  TEST_VERIFY (errno == 0);
+
+  /* Exercise process termination.   */
+  exit (0);
+}
+
+/* getaddrinfo test.  To be run from a subprocess.  */
+static void
+test_gai (int family)
+{
+  struct addrinfo hints =
+    {
+      .ai_family = family,
+      .ai_protocol = IPPROTO_TCP,
+      .ai_socktype = SOCK_STREAM,
+    };
+
+  struct addrinfo *ai;
+  int ret = getaddrinfo ("host2.example.com", "80", &hints, &ai);
+  check_addrinfo ("host2.example.com", ai, ret,
+                  "address: STREAM/TCP 192.0.2.2 80\n"
+                  "address: STREAM/TCP 192.0.2.1 80\n");
+
+  ret = getaddrinfo ("host5.example.com", "80", &hints, &ai);
+  check_addrinfo ("host5.example.com", ai, ret,
+                  "address: STREAM/TCP 192.0.2.1 80\n"
+                  "address: STREAM/TCP 192.0.2.5 80\n");
+
+  ret = getaddrinfo ("www.example.com", "80", &hints, &ai);
+  check_addrinfo ("www.example.com", ai, ret,
+                  "address: STREAM/TCP 192.0.2.80 80\n");
+
+  ret = getaddrinfo ("www1.example.com", "80", &hints, &ai);
+  check_addrinfo ("www1.example.com", ai, ret,
+                  "address: STREAM/TCP 192.0.2.81 80\n");
+}
+
+/* Subprocess routine for gethostbyname/getaddrinfo testing.  */
+static void
+subprocess_gethost (void *closure)
+{
+  xchroot (chroot_env->path_chroot);
+
+  /* This tests enlarging the read buffer in the multi case.  */
+  struct xmemstream expected;
+  xopen_memstream (&expected);
+  fputs ("name: host2.example.com\n", expected.out);
+  for (int j = 1; j < name_count; ++j)
+    /* NB: host2 is duplicated in the alias list.  */
+    fprintf (expected.out, "alias: host%d.example.com\n", j);
+  fputs ("alias: host0.example.com\n"
+         "address: 192.0.2.2\n"
+         "address: 192.0.2.1\n",
+         expected.out);
+  xfclose_memstream (&expected);
+  check_hostent ("host2.example.com",
+                 gethostbyname ("host2.example.com"),
+                 expected.buffer);
+  free (expected.buffer);
+
+  /* Similarly, but with a different order in the /etc/hosts file.  */
+  xopen_memstream (&expected);
+  fputs ("name: host0.example.com\n", expected.out);
+  for (int j = 1; j < name_count; ++j)
+    fprintf (expected.out, "alias: host%d.example.com\n", j);
+  /* NB: host5 is duplicated in the alias list.  */
+  fputs ("alias: host5.example.com\n"
+         "address: 192.0.2.1\n"
+         "address: 192.0.2.5\n",
+         expected.out);
+  xfclose_memstream (&expected);
+  check_hostent ("host5.example.com",
+                 gethostbyname ("host5.example.com"),
+                 expected.buffer);
+  free (expected.buffer);
+
+  check_hostent ("www.example.com",
+                 gethostbyname ("www.example.com"),
+                 host4_expected);
+  check_hostent ("www1.example.com",
+                 gethostbyname ("www1.example.com"),
+                 host6_expected);
+
+  test_gai (AF_INET);
+  test_gai (AF_UNSPEC);
+}
+
+static int
+do_test (void)
+{
+  support_become_root ();
+  if (!support_can_chroot ())
+    return EXIT_UNSUPPORTED;
+
+  __nss_configure_lookup ("hosts", "files");
+  if (dlopen (LIBNSS_FILES_SO, RTLD_LAZY) == NULL)
+    FAIL_EXIT1 ("could not load " LIBNSS_DNS_SO ": %s", dlerror ());
+
+  /* Each name takes about 20 bytes, so this covers a wide range of
+     buffer sizes, from less than 1000 bytes to about 18000 bytes.  */
+  for (name_count = 40; name_count <= 850; ++name_count)
+    {
+      write_hosts ();
+
+      for (do_stayopen = -1; do_stayopen < 2; ++do_stayopen)
+        for (do_endent = 0; do_endent < 2; ++do_endent)
+          {
+            if (test_verbose > 0)
+              printf ("info: name_count=%d do_stayopen=%d do_endent=%d\n",
+                      name_count, do_stayopen, do_endent);
+            support_isolate_in_subprocess (subprocess_getent, NULL);
+          }
+
+      support_isolate_in_subprocess (subprocess_gethost, NULL);
+    }
+
+  support_chroot_free (chroot_env);
+  return 0;
+}
+
+#define PREPARE prepare
+#include <support/test-driver.c>