| Message ID | 87a7k1531v.fsf@oldenburg2.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | nss_files: Fix /etc/aliases null pointer dereference [BZ #24059] | expand |
On 1/15/19 10:54 AM, Florian Weimer wrote: > If /etc/aliases ends with a continuation line (a line that starts > with whitespace) which does not have a training newline character, > the file parser would crash due to a null pointer dereference. > > 2019-01-15 Florian Weimer <fweimer@redhat.com> > > [BZ #24059] > * nss/nss_files/files-alias.c (get_next_alias): Handle > continuation line without newline at the end. > * nss/tst-nss-files-alias-truncated.c: New file. > * nss/Makefile [$(build-shared)] (tests): Add > tst-nss-files-alias-truncated. > (tst-nss-files-alias-truncated): Link with libnss_files.so. > * support/namespace.h (struct support_chroot_configuration): Add > aliases member. > (struct support_chroot): Add path_aliases member. > * support/support_chroot.c (support_chroot_create): Handle > aliases. > (support_chroot_free): Free path_aliases. > OK for master. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > diff --git a/nss/Makefile b/nss/Makefile > index 0fa28f0c5e..e66e01d45a 100644 > --- a/nss/Makefile > +++ b/nss/Makefile > @@ -68,6 +68,7 @@ tests += tst-nss-files-hosts-erange > tests += tst-nss-files-hosts-multi > tests += tst-nss-files-hosts-getent > tests += tst-nss-files-alias-leak > +tests += tst-nss-files-alias-truncated OK. > endif > > # If we have a thread library then we can test cancellation against > @@ -176,3 +177,4 @@ $(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 > +$(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so > diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c > index 540eb9c678..34d7aa84cc 100644 > --- a/nss/nss_files/files-alias.c > +++ b/nss/nss_files/files-alias.c > @@ -333,6 +333,16 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result, > can be ignored. */ > first_unused[room_left - 1] = '\xff'; > line = fgets_unlocked (first_unused, room_left, stream); > + if (line == NULL) > + { > + /* Continuation line without any data and > + without a newline at the end. Treat it as an > + empty line and retry, reaching EOF once > + more. */ > + line = first_unused; > + *line = '\0'; > + continue; > + } OK, so this allows us to process the previously read line as a whole instead of throwing it away because of a truncated continuation. Setting line to first_unused and terminating is OK. > if (first_unused[room_left - 1] != '\xff') > goto no_more_room; > cp = strpbrk (line, "#\n"); > diff --git a/nss/tst-nss-files-alias-truncated.c b/nss/tst-nss-files-alias-truncated.c > new file mode 100644 > index 0000000000..2d6aba3c0e > --- /dev/null > +++ b/nss/tst-nss-files-alias-truncated.c > @@ -0,0 +1,66 @@ > +/* Check handling of missing end-of-line at end of /etc/aliases (bug 24059). OK. My preference would be a test-container test that doesn't have all of this boiler plate, but I leave that choice up to you. > + Copyright (C) 2019 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 <nss.h> > +#include <stddef.h> > +#include <support/check.h> > +#include <support/namespace.h> > +#include <support/test-driver.h> > +#include <support/xunistd.h> > + > +static void > +in_chroot (void *closure) > +{ > + struct support_chroot *chroot_env = closure; > + xchroot (chroot_env->path_chroot); > + > + struct aliasent *e = getaliasbyname ("user1"); > + TEST_VERIFY_EXIT (e != NULL); > + TEST_COMPARE_STRING (e->alias_name, "user1"); > + TEST_COMPARE (e->alias_members_len, 1); > + TEST_VERIFY_EXIT (e->alias_members != NULL); > + TEST_COMPARE_STRING (e->alias_members[0], "alias1"); > + TEST_VERIFY (e->alias_local); OK. > +} > + > +static int > +do_test (void) > +{ > + /* nss_files has already been loaded via DT_NEEDED, outside the > + chroot. */ > + __nss_configure_lookup ("aliases", "files"); > + > + support_become_root (); > + if (!support_can_chroot ()) > + return EXIT_UNSUPPORTED; > + > + struct support_chroot *chroot_env = support_chroot_create > + ((struct support_chroot_configuration) > + { > + .aliases = "user1: alias1,\n" > + " " /* Continuation line, but no \n. */ OK. > + }); > + > + support_isolate_in_subprocess (in_chroot, chroot_env); > + > + support_chroot_free (chroot_env); > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/support/namespace.h b/support/namespace.h > index f93754934f..4770cf9e90 100644 > --- a/support/namespace.h > +++ b/support/namespace.h > @@ -74,6 +74,7 @@ struct support_chroot_configuration > const char *resolv_conf; /* /etc/resolv.conf. */ > const char *hosts; /* /etc/hosts. */ > const char *host_conf; /* /etc/host.conf. */ > + const char *aliases; /* /etc/aliases. */ OK. > }; > > /* The result of the creation of a chroot. */ > @@ -90,6 +91,7 @@ struct support_chroot > char *path_resolv_conf; /* /etc/resolv.conf. */ > char *path_hosts; /* /etc/hosts. */ > char *path_host_conf; /* /etc/host.conf. */ > + char *path_aliases; /* /etc/aliases. */ OK. > }; > > /* Create a chroot environment. The returned data should be freed > diff --git a/support/support_chroot.c b/support/support_chroot.c > index b866b89a18..412ec33f53 100644 > --- a/support/support_chroot.c > +++ b/support/support_chroot.c > @@ -56,6 +56,7 @@ support_chroot_create (struct support_chroot_configuration conf) > &chroot->path_resolv_conf); > write_file (path_etc, "hosts", conf.hosts, &chroot->path_hosts); > write_file (path_etc, "host.conf", conf.host_conf, &chroot->path_host_conf); > + write_file (path_etc, "aliases", conf.aliases, &chroot->path_aliases); OK. > > free (path_etc); > > @@ -77,5 +78,6 @@ support_chroot_free (struct support_chroot *chroot) > free (chroot->path_resolv_conf); > free (chroot->path_hosts); > free (chroot->path_host_conf); > + free (chroot->path_aliases); OK. > free (chroot); > } >
On 15/01/2019 15:54, Florian Weimer wrote: > If /etc/aliases ends with a continuation line (a line that starts > with whitespace) which does not have a training newline character, > the file parser would crash due to a null pointer dereference. > > 2019-01-15 Florian Weimer <fweimer@redhat.com> > > [BZ #24059] > * nss/nss_files/files-alias.c (get_next_alias): Handle > continuation line without newline at the end. > * nss/tst-nss-files-alias-truncated.c: New file. > * nss/Makefile [$(build-shared)] (tests): Add > tst-nss-files-alias-truncated. > (tst-nss-files-alias-truncated): Link with libnss_files.so. > * support/namespace.h (struct support_chroot_configuration): Add > aliases member. > (struct support_chroot): Add path_aliases member. > * support/support_chroot.c (support_chroot_create): Handle > aliases. > (support_chroot_free): Free path_aliases. aarch64 buildbot consistently fails with FAIL: nss/tst-nss-files-alias-truncated error: tst-nss-files-alias-truncated.c:34: not true: e != NULL error: support_isolate_in_subprocess.c:37: child process exited with status 256 error: 2 test failures i'm not yet sure why (passes with ./testrun.sh)
* Szabolcs Nagy: > On 15/01/2019 15:54, Florian Weimer wrote: >> If /etc/aliases ends with a continuation line (a line that starts >> with whitespace) which does not have a training newline character, >> the file parser would crash due to a null pointer dereference. >> >> 2019-01-15 Florian Weimer <fweimer@redhat.com> >> >> [BZ #24059] >> * nss/nss_files/files-alias.c (get_next_alias): Handle >> continuation line without newline at the end. >> * nss/tst-nss-files-alias-truncated.c: New file. >> * nss/Makefile [$(build-shared)] (tests): Add >> tst-nss-files-alias-truncated. >> (tst-nss-files-alias-truncated): Link with libnss_files.so. >> * support/namespace.h (struct support_chroot_configuration): Add >> aliases member. >> (struct support_chroot): Add path_aliases member. >> * support/support_chroot.c (support_chroot_create): Handle >> aliases. >> (support_chroot_free): Free path_aliases. > > aarch64 buildbot consistently fails with > > FAIL: nss/tst-nss-files-alias-truncated > > error: tst-nss-files-alias-truncated.c:34: not true: e != NULL > error: support_isolate_in_subprocess.c:37: child process exited with status 256 > error: 2 test failures > > i'm not yet sure why (passes with ./testrun.sh) I haven't seen that, and I don't know what could be causing it. I fear you have to debug it in the context of the build bot, sorry. Thanks, Florian
On 01/02/2019 15:15, Florian Weimer wrote: > * Szabolcs Nagy: > >> On 15/01/2019 15:54, Florian Weimer wrote: >>> If /etc/aliases ends with a continuation line (a line that starts >>> with whitespace) which does not have a training newline character, >>> the file parser would crash due to a null pointer dereference. >>> >>> 2019-01-15 Florian Weimer <fweimer@redhat.com> >>> >>> [BZ #24059] >>> * nss/nss_files/files-alias.c (get_next_alias): Handle >>> continuation line without newline at the end. >>> * nss/tst-nss-files-alias-truncated.c: New file. >>> * nss/Makefile [$(build-shared)] (tests): Add >>> tst-nss-files-alias-truncated. >>> (tst-nss-files-alias-truncated): Link with libnss_files.so. >>> * support/namespace.h (struct support_chroot_configuration): Add >>> aliases member. >>> (struct support_chroot): Add path_aliases member. >>> * support/support_chroot.c (support_chroot_create): Handle >>> aliases. >>> (support_chroot_free): Free path_aliases. >> >> aarch64 buildbot consistently fails with >> >> FAIL: nss/tst-nss-files-alias-truncated >> >> error: tst-nss-files-alias-truncated.c:34: not true: e != NULL >> error: support_isolate_in_subprocess.c:37: child process exited with status 256 >> error: 2 test failures >> >> i'm not yet sure why (passes with ./testrun.sh) > > I haven't seen that, and I don't know what could be causing it. I fear > you have to debug it in the context of the build bot, sorry. libnss_files.so.2 is not listed as explicit dependency for some reason so it is not loaded at program startup, but after chroot is entered, but it's not available in the chroot so the first api call fails. (the buildbot used a gcc-5 toolchain so now that we require gcc-6 the aarch64 buildbot is dead anyway.)
* Szabolcs Nagy: > On 01/02/2019 15:15, Florian Weimer wrote: >> * Szabolcs Nagy: >> >>> On 15/01/2019 15:54, Florian Weimer wrote: >>>> If /etc/aliases ends with a continuation line (a line that starts >>>> with whitespace) which does not have a training newline character, >>>> the file parser would crash due to a null pointer dereference. >>>> >>>> 2019-01-15 Florian Weimer <fweimer@redhat.com> >>>> >>>> [BZ #24059] >>>> * nss/nss_files/files-alias.c (get_next_alias): Handle >>>> continuation line without newline at the end. >>>> * nss/tst-nss-files-alias-truncated.c: New file. >>>> * nss/Makefile [$(build-shared)] (tests): Add >>>> tst-nss-files-alias-truncated. >>>> (tst-nss-files-alias-truncated): Link with libnss_files.so. >>>> * support/namespace.h (struct support_chroot_configuration): Add >>>> aliases member. >>>> (struct support_chroot): Add path_aliases member. >>>> * support/support_chroot.c (support_chroot_create): Handle >>>> aliases. >>>> (support_chroot_free): Free path_aliases. >>> >>> aarch64 buildbot consistently fails with >>> >>> FAIL: nss/tst-nss-files-alias-truncated >>> >>> error: tst-nss-files-alias-truncated.c:34: not true: e != NULL >>> error: support_isolate_in_subprocess.c:37: child process exited with status 256 >>> error: 2 test failures >>> >>> i'm not yet sure why (passes with ./testrun.sh) >> >> I haven't seen that, and I don't know what could be causing it. I fear >> you have to debug it in the context of the build bot, sorry. > > libnss_files.so.2 is not listed as explicit dependency > for some reason so it is not loaded at program startup, > but after chroot is entered, but it's not available in > the chroot so the first api call fails. There's already this in nss/Makefile: $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so I expected this causes a DT_NEEDED entry to be added to the executable, so it is loaded upon startup, outside of the chroot. Thanks, Florian
* Florian Weimer <fweimer@redhat.com> [2019-02-01 18:32:23 +0100]: > * Szabolcs Nagy: > > > > libnss_files.so.2 is not listed as explicit dependency > > for some reason so it is not loaded at program startup, > > but after chroot is entered, but it's not available in > > the chroot so the first api call fails. > > There's already this in nss/Makefile: > > $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so > > I expected this causes a DT_NEEDED entry to be added to the executable, > so it is loaded upon startup, outside of the chroot. it seems debian/ubuntu gcc always passes --as-needed to the linker https://wiki.debian.org/ToolChain/DSOLinking so either a reference is needed to the lib or -Wl,--no-as-needed lib -Wl,--as-needed ldflag to force a DT_NEEDED. ..or copy the lib to the chroot.
* Szabolcs Nagy: > * Florian Weimer <fweimer@redhat.com> [2019-02-01 18:32:23 +0100]: >> * Szabolcs Nagy: >> > >> > libnss_files.so.2 is not listed as explicit dependency >> > for some reason so it is not loaded at program startup, >> > but after chroot is entered, but it's not available in >> > the chroot so the first api call fails. >> >> There's already this in nss/Makefile: >> >> $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so >> >> I expected this causes a DT_NEEDED entry to be added to the executable, >> so it is loaded upon startup, outside of the chroot. > > it seems debian/ubuntu gcc always passes --as-needed to the linker > https://wiki.debian.org/ToolChain/DSOLinking Nice find. Yes, that completely explains it. > so either a reference is needed to the lib or > -Wl,--no-as-needed lib -Wl,--as-needed > ldflag to force a DT_NEEDED. Hmm. Should we put this on the general linker command line for tests? Are there any cases where we actually want --as-needed behavior for objects linked into tests? > ..or copy the lib to the chroot. The test-in-container framework does not work on our currrent reference machines, even as root. 8-( In other tests, I have used dlopen before the chroot. I could adjust this test accordingly, but it adds needless complexity in my opinion.
* Szabolcs Nagy: > * Florian Weimer <fweimer@redhat.com> [2019-02-01 18:32:23 +0100]: >> * Szabolcs Nagy: >> > >> > libnss_files.so.2 is not listed as explicit dependency >> > for some reason so it is not loaded at program startup, >> > but after chroot is entered, but it's not available in >> > the chroot so the first api call fails. >> >> There's already this in nss/Makefile: >> >> $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so >> >> I expected this causes a DT_NEEDED entry to be added to the executable, >> so it is loaded upon startup, outside of the chroot. > > it seems debian/ubuntu gcc always passes --as-needed to the linker > https://wiki.debian.org/ToolChain/DSOLinking > > so either a reference is needed to the lib or > -Wl,--no-as-needed lib -Wl,--as-needed > ldflag to force a DT_NEEDED. > > ..or copy the lib to the chroot. So what should we do here? Should we disable --as-needed across the board? Or should we fix the test? Thanks, Florian
On 02/02/2019 15:03, Florian Weimer wrote: > * Szabolcs Nagy: > >> * Florian Weimer <fweimer@redhat.com> [2019-02-01 18:32:23 +0100]: >>> * Szabolcs Nagy: >>>> >>>> libnss_files.so.2 is not listed as explicit dependency >>>> for some reason so it is not loaded at program startup, >>>> but after chroot is entered, but it's not available in >>>> the chroot so the first api call fails. >>> >>> There's already this in nss/Makefile: >>> >>> $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so >>> >>> I expected this causes a DT_NEEDED entry to be added to the executable, >>> so it is loaded upon startup, outside of the chroot. >> >> it seems debian/ubuntu gcc always passes --as-needed to the linker >> https://wiki.debian.org/ToolChain/DSOLinking > > Nice find. Yes, that completely explains it. > >> so either a reference is needed to the lib or >> -Wl,--no-as-needed lib -Wl,--as-needed >> ldflag to force a DT_NEEDED. > > Hmm. Should we put this on the general linker command line for tests? > Are there any cases where we actually want --as-needed behavior for > objects linked into tests? > >> ..or copy the lib to the chroot. > > The test-in-container framework does not work on our currrent reference > machines, even as root. 8-( > > In other tests, I have used dlopen before the chroot. I could adjust > this test accordingly, but it adds needless complexity in my opinion. > What is preventing the tests to work with test-in-container by copying the lib in the chroot?
On 13/03/2019 16:36, Florian Weimer wrote: > * Szabolcs Nagy: > >> * Florian Weimer <fweimer@redhat.com> [2019-02-01 18:32:23 +0100]: >>> * Szabolcs Nagy: >>>> >>>> libnss_files.so.2 is not listed as explicit dependency >>>> for some reason so it is not loaded at program startup, >>>> but after chroot is entered, but it's not available in >>>> the chroot so the first api call fails. >>> >>> There's already this in nss/Makefile: >>> >>> $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so >>> >>> I expected this causes a DT_NEEDED entry to be added to the executable, >>> so it is loaded upon startup, outside of the chroot. >> >> it seems debian/ubuntu gcc always passes --as-needed to the linker >> https://wiki.debian.org/ToolChain/DSOLinking >> >> so either a reference is needed to the lib or >> -Wl,--no-as-needed lib -Wl,--as-needed >> ldflag to force a DT_NEEDED. >> >> ..or copy the lib to the chroot. > > So what should we do here? > > Should we disable --as-needed across the board? Or should we fix the > test? ideally the chroot should have all the runtime libs that may be loaded, if that's too big hassle, then i'd fix the test (to enforce DT_NEEDED or use dlopen before chroot). (this issue affects the aarch64 buildbot, but that's not running since the minimum gcc requirement got increased, i'll ask for a buildbot server restart)
* Adhemerval Zanella: > What is preventing the tests to work with test-in-container by copying > the lib in the chroot? For test-in-container, I'm still trying to figure out how to reliably re-run a test after a test update, without having to wait five minutes or more, until a rebuild from nothing completes. Thanks, Florian
* Szabolcs Nagy: > On 13/03/2019 16:36, Florian Weimer wrote: >> * Szabolcs Nagy: >> >>> * Florian Weimer <fweimer@redhat.com> [2019-02-01 18:32:23 +0100]: >>>> * Szabolcs Nagy: >>>>> >>>>> libnss_files.so.2 is not listed as explicit dependency >>>>> for some reason so it is not loaded at program startup, >>>>> but after chroot is entered, but it's not available in >>>>> the chroot so the first api call fails. >>>> >>>> There's already this in nss/Makefile: >>>> >>>> $(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so >>>> >>>> I expected this causes a DT_NEEDED entry to be added to the executable, >>>> so it is loaded upon startup, outside of the chroot. >>> >>> it seems debian/ubuntu gcc always passes --as-needed to the linker >>> https://wiki.debian.org/ToolChain/DSOLinking >>> >>> so either a reference is needed to the lib or >>> -Wl,--no-as-needed lib -Wl,--as-needed >>> ldflag to force a DT_NEEDED. >>> >>> ..or copy the lib to the chroot. >> >> So what should we do here? >> >> Should we disable --as-needed across the board? Or should we fix the >> test? > > ideally the chroot should have all the runtime libs that > may be loaded, if that's too big hassle, then i'd fix > the test (to enforce DT_NEEDED or use dlopen before chroot). Let's do it via dlopen. See below. Thanks, Florian nss: Fix tst-nss-files-alias-truncated for default --as-needed linking Linking to the NSS module directly does not work if the linker defaults to --as-needed because it will remove the apparently unused DSO reference and not generate a DT_NEEDED entry. Use an explicit dlopen call, like in the other chroot tests involving NSS modules. 2019-03-14 Florian Weimer <fweimer@redhat.com> * nss/tst-nss-files-alias-truncated.c (do_test): Load libnss_files. * nss/Makefile (tst-nss-files-alias-truncated): Link with -ldl, but not with libnss_files. (tst-nss-files-alias-truncated.out): Depend on libnss_files. diff --git a/nss/Makefile b/nss/Makefile index a8caa8af38..95081bddc5 100644 --- a/nss/Makefile +++ b/nss/Makefile @@ -178,4 +178,5 @@ $(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 -$(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so +$(objpfx)tst-nss-files-alias-truncated: $(libdl) +$(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so diff --git a/nss/tst-nss-files-alias-truncated.c b/nss/tst-nss-files-alias-truncated.c index 2d6aba3c0e..029ae6a2a7 100644 --- a/nss/tst-nss-files-alias-truncated.c +++ b/nss/tst-nss-files-alias-truncated.c @@ -17,11 +17,13 @@ <http://www.gnu.org/licenses/>. */ #include <aliases.h> +#include <gnu/lib-names.h> #include <nss.h> #include <stddef.h> #include <support/check.h> #include <support/namespace.h> #include <support/test-driver.h> +#include <support/xdlfcn.h> #include <support/xunistd.h> static void @@ -42,8 +44,9 @@ in_chroot (void *closure) static int do_test (void) { - /* nss_files has already been loaded via DT_NEEDED, outside the - chroot. */ + /* Make sure we don't try to load the module in the chroot. */ + xdlopen (LIBNSS_FILES_SO, RTLD_NOW); + __nss_configure_lookup ("aliases", "files"); support_become_root ();
On 14/03/2019 14:04, Florian Weimer wrote: > * Szabolcs Nagy: >> ideally the chroot should have all the runtime libs that >> may be loaded, if that's too big hassle, then i'd fix >> the test (to enforce DT_NEEDED or use dlopen before chroot). > > Let's do it via dlopen. See below. > > Thanks, > Florian > > nss: Fix tst-nss-files-alias-truncated for default --as-needed linking > > Linking to the NSS module directly does not work if the linker defaults > to --as-needed because it will remove the apparently unused DSO > reference and not generate a DT_NEEDED entry. Use an explicit dlopen > call, like in the other chroot tests involving NSS modules. > > 2019-03-14 Florian Weimer <fweimer@redhat.com> > > * nss/tst-nss-files-alias-truncated.c (do_test): Load > libnss_files. > * nss/Makefile (tst-nss-files-alias-truncated): Link with -ldl, > but not with libnss_files. > (tst-nss-files-alias-truncated.out): Depend on libnss_files. looks reasonable. thanks.
diff --git a/nss/Makefile b/nss/Makefile index 0fa28f0c5e..e66e01d45a 100644 --- a/nss/Makefile +++ b/nss/Makefile @@ -68,6 +68,7 @@ tests += tst-nss-files-hosts-erange tests += tst-nss-files-hosts-multi tests += tst-nss-files-hosts-getent tests += tst-nss-files-alias-leak +tests += tst-nss-files-alias-truncated endif # If we have a thread library then we can test cancellation against @@ -176,3 +177,4 @@ $(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 +$(objpfx)tst-nss-files-alias-truncated: $(objpfx)/libnss_files.so diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c index 540eb9c678..34d7aa84cc 100644 --- a/nss/nss_files/files-alias.c +++ b/nss/nss_files/files-alias.c @@ -333,6 +333,16 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result, can be ignored. */ first_unused[room_left - 1] = '\xff'; line = fgets_unlocked (first_unused, room_left, stream); + if (line == NULL) + { + /* Continuation line without any data and + without a newline at the end. Treat it as an + empty line and retry, reaching EOF once + more. */ + line = first_unused; + *line = '\0'; + continue; + } if (first_unused[room_left - 1] != '\xff') goto no_more_room; cp = strpbrk (line, "#\n"); diff --git a/nss/tst-nss-files-alias-truncated.c b/nss/tst-nss-files-alias-truncated.c new file mode 100644 index 0000000000..2d6aba3c0e --- /dev/null +++ b/nss/tst-nss-files-alias-truncated.c @@ -0,0 +1,66 @@ +/* Check handling of missing end-of-line at end of /etc/aliases (bug 24059). + Copyright (C) 2019 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 <nss.h> +#include <stddef.h> +#include <support/check.h> +#include <support/namespace.h> +#include <support/test-driver.h> +#include <support/xunistd.h> + +static void +in_chroot (void *closure) +{ + struct support_chroot *chroot_env = closure; + xchroot (chroot_env->path_chroot); + + struct aliasent *e = getaliasbyname ("user1"); + TEST_VERIFY_EXIT (e != NULL); + TEST_COMPARE_STRING (e->alias_name, "user1"); + TEST_COMPARE (e->alias_members_len, 1); + TEST_VERIFY_EXIT (e->alias_members != NULL); + TEST_COMPARE_STRING (e->alias_members[0], "alias1"); + TEST_VERIFY (e->alias_local); +} + +static int +do_test (void) +{ + /* nss_files has already been loaded via DT_NEEDED, outside the + chroot. */ + __nss_configure_lookup ("aliases", "files"); + + support_become_root (); + if (!support_can_chroot ()) + return EXIT_UNSUPPORTED; + + struct support_chroot *chroot_env = support_chroot_create + ((struct support_chroot_configuration) + { + .aliases = "user1: alias1,\n" + " " /* Continuation line, but no \n. */ + }); + + support_isolate_in_subprocess (in_chroot, chroot_env); + + support_chroot_free (chroot_env); + return 0; +} + +#include <support/test-driver.c> diff --git a/support/namespace.h b/support/namespace.h index f93754934f..4770cf9e90 100644 --- a/support/namespace.h +++ b/support/namespace.h @@ -74,6 +74,7 @@ struct support_chroot_configuration const char *resolv_conf; /* /etc/resolv.conf. */ const char *hosts; /* /etc/hosts. */ const char *host_conf; /* /etc/host.conf. */ + const char *aliases; /* /etc/aliases. */ }; /* The result of the creation of a chroot. */ @@ -90,6 +91,7 @@ struct support_chroot char *path_resolv_conf; /* /etc/resolv.conf. */ char *path_hosts; /* /etc/hosts. */ char *path_host_conf; /* /etc/host.conf. */ + char *path_aliases; /* /etc/aliases. */ }; /* Create a chroot environment. The returned data should be freed diff --git a/support/support_chroot.c b/support/support_chroot.c index b866b89a18..412ec33f53 100644 --- a/support/support_chroot.c +++ b/support/support_chroot.c @@ -56,6 +56,7 @@ support_chroot_create (struct support_chroot_configuration conf) &chroot->path_resolv_conf); write_file (path_etc, "hosts", conf.hosts, &chroot->path_hosts); write_file (path_etc, "host.conf", conf.host_conf, &chroot->path_host_conf); + write_file (path_etc, "aliases", conf.aliases, &chroot->path_aliases); free (path_etc); @@ -77,5 +78,6 @@ support_chroot_free (struct support_chroot *chroot) free (chroot->path_resolv_conf); free (chroot->path_hosts); free (chroot->path_host_conf); + free (chroot->path_aliases); free (chroot); }