nss_files: Fix /etc/aliases null pointer dereference [BZ #24059]

Message ID 87k1h15z7g.fsf@oldenburg2.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer March 14, 2019, 2:04 p.m. UTC
  * 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.
  

Comments

Szabolcs Nagy March 14, 2019, 2:09 p.m. UTC | #1
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.
  

Patch

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 ();