Cherry-pick from release/2.28/master upstream.

- nss_files: Fix file stream leak in aliases lookup (swbz#23521)
- nscd: Deallocate existing user names in file parser.
- error, error_at_line: Add missing va_end calls.
This commit is contained in:
Carlos O'Donell 2018-08-25 13:37:06 -04:00
parent bebdebbdc3
commit 065eb95ad4
4 changed files with 378 additions and 1 deletions

28
glibc-error-va_end.patch Normal file
View File

@ -0,0 +1,28 @@
commit d05b05d1570ba3ae354a2f5a3cfeefb373b09979
Author: Florian Weimer <fweimer@redhat.com>
Date: Mon Aug 13 14:28:07 2018 +0200
error, error_at_line: Add missing va_end calls
(cherry picked from commit b7b52b9dec337a08a89bc67638773be652eba332)
diff --git a/misc/error.c b/misc/error.c
index b4e8b6c93886b737..03378e2f2aa6251e 100644
--- a/misc/error.c
+++ b/misc/error.c
@@ -319,6 +319,7 @@ error (int status, int errnum, const char *message, ...)
va_start (args, message);
error_tail (status, errnum, message, args);
+ va_end (args);
#ifdef _LIBC
_IO_funlockfile (stderr);
@@ -390,6 +391,7 @@ error_at_line (int status, int errnum, const char *file_name,
va_start (args, message);
error_tail (status, errnum, message, args);
+ va_end (args);
#ifdef _LIBC
_IO_funlockfile (stderr);

35
glibc-nscd-leak.patch Normal file
View File

@ -0,0 +1,35 @@
commit bfcfa22589f2b4277a65e60c6b736b6bbfbd87d0
Author: Florian Weimer <fweimer@redhat.com>
Date: Tue Aug 14 10:51:07 2018 +0200
nscd: Deallocate existing user names in file parser
This avoids a theoretical memory leak (theoretical because it depends on
multiple server-user/stat-user directives in the configuration file).
(cherry picked from commit 2d7acfac3ebf266dcbc82d0d6cc576f626953a03)
diff --git a/nscd/nscd_conf.c b/nscd/nscd_conf.c
index 265a02434dd26c29..7293b795b6bcf71e 100644
--- a/nscd/nscd_conf.c
+++ b/nscd/nscd_conf.c
@@ -190,7 +190,10 @@ nscd_parse_file (const char *fname, struct database_dyn dbs[lastdb])
if (!arg1)
error (0, 0, _("Must specify user name for server-user option"));
else
- server_user = xstrdup (arg1);
+ {
+ free ((char *) server_user);
+ server_user = xstrdup (arg1);
+ }
}
else if (strcmp (entry, "stat-user") == 0)
{
@@ -198,6 +201,7 @@ nscd_parse_file (const char *fname, struct database_dyn dbs[lastdb])
error (0, 0, _("Must specify user name for stat-user option"));
else
{
+ free ((char *) stat_user);
stat_user = xstrdup (arg1);
struct passwd *pw = getpwnam (stat_user);

306
glibc-nss_files-leak.patch Normal file
View File

@ -0,0 +1,306 @@
commit 2f498f3d140ab5152bd784df2be7af7d9c5e63ed
Author: Florian Weimer <fweimer@redhat.com>
Date: Tue Aug 14 10:57:48 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.
(cherry picked from commit e95c6f61920a0f9237cfb292fa44ad500e1df09b)
diff --git a/nss/Makefile b/nss/Makefile
index 66fac7f5b8a4c0d8..5209fc0456dd6786 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 cfd34b66b921bbff..35b0bfc5d2479ab6 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 0000000000000000..26d38e2dba1ddaf3
--- /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>

View File

@ -1,6 +1,6 @@
%define glibcsrcdir glibc-2.28
%define glibcversion 2.28
%define glibcrelease 5%{?dist}
%define glibcrelease 6%{?dist}
# Pre-release tarballs are pulled in from git using a command that is
# effectively:
#
@ -162,6 +162,9 @@ Patch25: glibc-asflags.patch
Patch26: glibc-ldflags.patch
Patch27: glibc-rh1614705.patch
Patch28: glibc-rh1615608.patch
Patch29: glibc-error-va_end.patch
Patch30: glibc-nscd-leak.patch
Patch31: glibc-nss_files-leak.patch
##############################################################################
# Continued list of core "glibc" package information:
@ -1877,6 +1880,11 @@ fi
%endif
%changelog
* Tue Aug 25 2018 Carlos O'Donell <carlos@redhat.com> - 2.28-6
- nss_files: Fix file stream leak in aliases lookup (swbz#23521)
- nscd: Deallocate existing user names in file parser.
- error, error_at_line: Add missing va_end calls.
* Mon Aug 13 2018 Carlos O'Donell <carlos@redhat.com> - 2.28-5
- Remove abort() warning in manual (#1615608)