This is the mail archive of the glibc-cvs@sourceware.org mailing list for the glibc 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]

GNU C Library master sources branch master updated. glibc-2.28.9000-56-ge95c6f6


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  e95c6f61920a0f9237cfb292fa44ad500e1df09b (commit)
      from  2d7acfac3ebf266dcbc82d0d6cc576f626953a03 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=e95c6f61920a0f9237cfb292fa44ad500e1df09b

commit e95c6f61920a0f9237cfb292fa44ad500e1df09b
Author: Florian Weimer <fweimer@redhat.com>
Date:   Tue Aug 14 10:52:06 2018 +0200

    nss_files: Fix file stream leak in aliases lookup [BZ #23521]
    
    In order to get a clean test case, it was necessary to fix partially
    fixed bug 23522 as well.

diff --git a/ChangeLog b/ChangeLog
index bf3c133..6ed43e0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2018-08-14  Florian Weimer  <fweimer@redhat.com>
 
+	[BZ #23521]
+	[BZ #23522]
+	* nss/nss_files/files-alias.c (get_next_alias): During :include:
+	processing, bail out if no room, and close the stream before
+	returning ERANGE.
+	* nss/Makefile (tests): Add tst-nss-files-alias-leak.
+	(tst-nss-files-alias-leak): Link with libdl.
+	(tst-nss-files-alias-leak.out): Depend on nss_files.
+
+	* nss/tst-nss-files-alias-leak.c: New file.
+
+2018-08-14  Florian Weimer  <fweimer@redhat.com>
+
 	* nscd/nscd_conf.c (nscd_parse_file): Deallocate old storage for
 	server_user, stat_user.
 
diff --git a/nss/Makefile b/nss/Makefile
index 66fac7f..5209fc0 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -65,6 +65,7 @@ ifeq (yes,$(build-shared))
 tests += tst-nss-files-hosts-erange
 tests += tst-nss-files-hosts-multi
 tests += tst-nss-files-hosts-getent
+tests += tst-nss-files-alias-leak
 endif
 
 # If we have a thread library then we can test cancellation against
@@ -171,3 +172,5 @@ endif
 $(objpfx)tst-nss-files-hosts-erange: $(libdl)
 $(objpfx)tst-nss-files-hosts-multi: $(libdl)
 $(objpfx)tst-nss-files-hosts-getent: $(libdl)
+$(objpfx)tst-nss-files-alias-leak: $(libdl)
+$(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c
index cfd34b6..35b0bfc 100644
--- a/nss/nss_files/files-alias.c
+++ b/nss/nss_files/files-alias.c
@@ -221,6 +221,13 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result,
 			{
 			  while (! feof_unlocked (listfile))
 			    {
+			      if (room_left < 2)
+				{
+				  free (old_line);
+				  fclose (listfile);
+				  goto no_more_room;
+				}
+
 			      first_unused[room_left - 1] = '\xff';
 			      line = fgets_unlocked (first_unused, room_left,
 						     listfile);
@@ -229,6 +236,7 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result,
 			      if (first_unused[room_left - 1] != '\xff')
 				{
 				  free (old_line);
+				  fclose (listfile);
 				  goto no_more_room;
 				}
 
@@ -256,6 +264,7 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result,
 						       + __alignof__ (char *)))
 					{
 					  free (old_line);
+					  fclose (listfile);
 					  goto no_more_room;
 					}
 				      room_left -= ((first_unused - cp)
diff --git a/nss/tst-nss-files-alias-leak.c b/nss/tst-nss-files-alias-leak.c
new file mode 100644
index 0000000..26d38e2
--- /dev/null
+++ b/nss/tst-nss-files-alias-leak.c
@@ -0,0 +1,237 @@
+/* Check for file descriptor leak in alias :include: processing (bug 23521).
+   Copyright (C) 2018 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 <aliases.h>
+#include <array_length.h>
+#include <dlfcn.h>
+#include <errno.h>
+#include <gnu/lib-names.h>
+#include <nss.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+static struct support_chroot *chroot_env;
+
+/* Number of the aliases for the "many" user.  This must be large
+   enough to trigger reallocation for the pointer array, but result in
+   answers below the maximum size tried in do_test.  */
+enum { many_aliases = 30 };
+
+static void
+prepare (int argc, char **argv)
+{
+  chroot_env = support_chroot_create
+    ((struct support_chroot_configuration) { } );
+
+  char *path = xasprintf ("%s/etc/aliases", chroot_env->path_chroot);
+  add_temp_file (path);
+  support_write_file_string
+    (path,
+     "user1: :include:/etc/aliases.user1\n"
+     "user2: :include:/etc/aliases.user2\n"
+     "comment: comment1, :include:/etc/aliases.comment\n"
+     "many: :include:/etc/aliases.many\n");
+  free (path);
+
+  path = xasprintf ("%s/etc/aliases.user1", chroot_env->path_chroot);
+  add_temp_file (path);
+  support_write_file_string (path, "alias1\n");
+  free (path);
+
+  path = xasprintf ("%s/etc/aliases.user2", chroot_env->path_chroot);
+  add_temp_file (path);
+  support_write_file_string (path, "alias1a, alias2\n");
+  free (path);
+
+  path = xasprintf ("%s/etc/aliases.comment", chroot_env->path_chroot);
+  add_temp_file (path);
+  support_write_file_string
+    (path,
+     /* The line must be longer than the line with the :include:
+        directive in /etc/aliases.  */
+     "# Long line.  ##############################################\n"
+     "comment2\n");
+  free (path);
+
+  path = xasprintf ("%s/etc/aliases.many", chroot_env->path_chroot);
+  add_temp_file (path);
+  FILE *fp = xfopen (path, "w");
+  for (int i = 0; i < many_aliases; ++i)
+    fprintf (fp, "a%d\n", i);
+  TEST_VERIFY_EXIT (! ferror (fp));
+  xfclose (fp);
+  free (path);
+}
+
+/* The names of the users to test.  */
+static const char *users[] = { "user1", "user2", "comment", "many" };
+
+static void
+check_aliases (int id, const struct aliasent *e)
+{
+  TEST_VERIFY_EXIT (id >= 0 || id < array_length (users));
+  const char *name = users[id];
+  TEST_COMPARE_BLOB (e->alias_name, strlen (e->alias_name),
+                     name, strlen (name));
+
+  switch (id)
+    {
+    case 0:
+      TEST_COMPARE (e->alias_members_len, 1);
+      TEST_COMPARE_BLOB (e->alias_members[0], strlen (e->alias_members[0]),
+                         "alias1", strlen ("alias1"));
+      break;
+
+    case 1:
+      TEST_COMPARE (e->alias_members_len, 2);
+      TEST_COMPARE_BLOB (e->alias_members[0], strlen (e->alias_members[0]),
+                         "alias1a", strlen ("alias1a"));
+      TEST_COMPARE_BLOB (e->alias_members[1], strlen (e->alias_members[1]),
+                         "alias2", strlen ("alias2"));
+      break;
+
+    case 2:
+      TEST_COMPARE (e->alias_members_len, 2);
+      TEST_COMPARE_BLOB (e->alias_members[0], strlen (e->alias_members[0]),
+                         "comment1", strlen ("comment1"));
+      TEST_COMPARE_BLOB (e->alias_members[1], strlen (e->alias_members[1]),
+                         "comment2", strlen ("comment2"));
+      break;
+
+    case 3:
+      TEST_COMPARE (e->alias_members_len, many_aliases);
+      for (int i = 0; i < e->alias_members_len; ++i)
+        {
+          char alias[30];
+          int len = snprintf (alias, sizeof (alias), "a%d", i);
+          TEST_VERIFY_EXIT (len > 0);
+          TEST_COMPARE_BLOB (e->alias_members[i], strlen (e->alias_members[i]),
+                             alias, len);
+        }
+      break;
+    }
+}
+
+static int
+do_test (void)
+{
+  /* Make sure we don't try to load the module in the chroot.  */
+  if (dlopen (LIBNSS_FILES_SO, RTLD_NOW) == NULL)
+    FAIL_EXIT1 ("could not load " LIBNSS_FILES_SO ": %s", dlerror ());
+
+  /* Some of these descriptors will become unavailable if there is a
+     file descriptor leak.  10 is chosen somewhat arbitrarily.  The
+     array must be longer than the number of files opened by nss_files
+     at the same time (currently that number is 2).  */
+  int next_descriptors[10];
+  for (size_t i = 0; i < array_length (next_descriptors); ++i)
+    {
+      next_descriptors[i] = dup (0);
+      TEST_VERIFY_EXIT (next_descriptors[i] > 0);
+    }
+  for (size_t i = 0; i < array_length (next_descriptors); ++i)
+    xclose (next_descriptors[i]);
+
+  support_become_root ();
+  if (!support_can_chroot ())
+    return EXIT_UNSUPPORTED;
+
+  __nss_configure_lookup ("aliases", "files");
+
+  xchroot (chroot_env->path_chroot);
+
+  /* Attempt various buffer sizes.  If the operation succeeds, we
+     expect correct data.  */
+  for (int id = 0; id < array_length (users); ++id)
+    {
+      bool found = false;
+      for (size_t size = 1; size <= 1000; ++size)
+        {
+          void *buffer = malloc (size);
+          struct aliasent result;
+          struct aliasent *res;
+          errno = EINVAL;
+          int ret = getaliasbyname_r (users[id], &result, buffer, size, &res);
+          if (ret == 0)
+            {
+              if (res != NULL)
+                {
+                  found = true;
+                  check_aliases (id, res);
+                }
+              else
+                {
+                  support_record_failure ();
+                  printf ("error: failed lookup for user \"%s\", size %zu\n",
+                          users[id], size);
+                }
+            }
+          else if (ret != ERANGE)
+            {
+              support_record_failure ();
+              printf ("error: invalid return code %d (user \%s\", size %zu)\n",
+                      ret, users[id], size);
+            }
+          free (buffer);
+
+          /* Make sure that we did not have a file descriptor leak.  */
+          for (size_t i = 0; i < array_length (next_descriptors); ++i)
+            {
+              int new_fd = dup (0);
+              if (new_fd != next_descriptors[i])
+                {
+                  support_record_failure ();
+                  printf ("error: descriptor %d at index %zu leaked"
+                          " (user \"%s\", size %zu)\n",
+                          next_descriptors[i], i, users[id], size);
+
+                  /* Close unexpected descriptor, the leak probing
+                     descriptors, and the leaked descriptor
+                     next_descriptors[i].  */
+                  xclose (new_fd);
+                  for (size_t j = 0; j <= i; ++j)
+                    xclose (next_descriptors[j]);
+                  goto next_size;
+                }
+            }
+          for (size_t i = 0; i < array_length (next_descriptors); ++i)
+            xclose (next_descriptors[i]);
+
+        next_size:
+          ;
+        }
+      if (!found)
+        {
+          support_record_failure ();
+          printf ("error: user %s not found\n", users[id]);
+        }
+    }
+
+  support_chroot_free (chroot_env);
+  return 0;
+}
+
+#define PREPARE prepare
+#include <support/test-driver.c>

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                      |   13 ++
 nss/Makefile                   |    3 +
 nss/nss_files/files-alias.c    |    9 ++
 nss/tst-nss-files-alias-leak.c |  237 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 nss/tst-nss-files-alias-leak.c


hooks/post-receive
-- 
GNU C Library master sources


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