[patch] Fix for crash in NSS when libc built with --enable-static-nss and exe built with -static

[Paul Pluzhnikov]

On Sun, Mar 18, 2012 at 6:58 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:

> Once the underlying change to make static NSS is in you should file a bug
> report in Bugzilla for the present issue, if there isn't one already.

Bug filed: http://sourceware.org/bugzilla/show_bug.cgi?id=13895

> You're evidently familiar with this code, ...

Sadly, I am not. As far as I can tell, this code goes all the way back
to this original commit (5f0e6fc7):

+Sun Jun 23 19:42:05 1996  Ulrich Drepper  <drepper@cygnus.com>
+
...
+       * nss/Makefile, nss/XXX-lookup.c, nss/file-lookup.c,
+       nss/getXXbyYY.c, nss/getXXbyYY_r.c, nss/getXXent.c,
+       nss/getXXent_r.c, nss/host-lookup.c, nss/network-lookup.c,
+       nss/nsswitch.c, nss/nsswitch.h, nss/proto-lookup.c,
+       nss/service-lookup.c: New files.  Implementation of name
+       service switch, following the approach in Solaris.

> Could you explain what the various values (ni->library->name, ni->name,
> fct_name) are supposed to be here (both for static-nss and the normal
> case), and why, therefore, the new code is correct and the old code isn't?

In the SHARED or !DO_STATIC_NSS case, ni->library is initialized by
nss_load_library, and as far as I can tell, ni->library->name is always
set to ni->name.

In the DO_STATIC_NSS case, nss_load_library is not called, and ni->library
stays NULL.

This suggests that we could replace all uses of ni->library->name with
ni->name and get rid of an extra memory reference.

I've added an "assert(ni->name == ni->library->name)" and ran "make check"
(both with and without --enable-static-nss), which didn't show any failures.

> Would the change ever have any effect for the normal (non-static-nss) case?

The original patch only touched code that is in the else clause of
"#if !defined DO_STATIC_NSS || defined SHARED"

Updated patch (attached) does change non-static-nss case, and also adds
a test case.

Thanks,
-- 
Paul Pluzhnikov

2012-03-26  Paul Pluzhnikov  <ppluzhnikov@google.com>

	[BZ #13895]
	* nss/nsswitch.c (nss_load_library, __nss_lookup_function): Avoid
	extra indirection.
	* nss/Makefile (tests-static, tests): Add tst-nss-static.
	* nss/tst-nss-static.c: New.
-------------- next part --------------
diff --git a/nss/Makefile b/nss/Makefile
index 54d50d0..a272ebe 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -75,6 +75,8 @@ libnss_db-inhibit-o	= $(filter-out .os,$(object-suffixes))
 ifeq ($(build-static-nss),yes)
 routines                += $(libnss_files-routines)
 static-only-routines    += $(libnss_files-routines)
+tests-static		= tst-nss-static
+tests			+= $(tests-static)
 endif
 
 include ../Rules
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 7acb140..26a5583 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1996-1999,2001-2007,2009,2010,2011
+/* Copyright (C) 1996-1999,2001-2007,2009,2010,2011,2012
    Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@cygnus.com>, 1996.
@@ -315,7 +315,7 @@ nss_load_library (service_user *ni)
   if (ni->library->lib_handle == NULL)
     {
       /* Load the shared library.  */
-      size_t shlen = (7 + strlen (ni->library->name) + 3
+      size_t shlen = (7 + strlen (ni->name) + 3
 		      + strlen (__nss_shlib_revision) + 1);
       int saved_errno = errno;
       char shlib_name[shlen];
@@ -323,7 +323,7 @@ nss_load_library (service_user *ni)
       /* Construct shared object name.  */
       __stpcpy (__stpcpy (__stpcpy (__stpcpy (shlib_name,
 					      "libnss_"),
-				    ni->library->name),
+				    ni->name),
 			  ".so"),
 		__nss_shlib_revision);
 
@@ -337,14 +337,14 @@ nss_load_library (service_user *ni)
       else if (is_nscd)
 	{
 	  /* Call the init function when nscd is used.  */
-	  size_t initlen = (5 + strlen (ni->library->name)
+	  size_t initlen = (5 + strlen (ni->name)
 			    + strlen ("_init") + 1);
 	  char init_name[initlen];
 
 	  /* Construct the init function name.  */
 	  __stpcpy (__stpcpy (__stpcpy (init_name,
 					"_nss_"),
-			      ni->library->name),
+			      ni->name),
 		    "_init");
 
 	  /* Find the optional init function.  */
@@ -428,13 +428,13 @@ __nss_lookup_function (service_user *ni, const char *fct_name)
 	  else
 	    {
 	      /* Get the desired function.  */
-	      size_t namlen = (5 + strlen (ni->library->name) + 1
+	      size_t namlen = (5 + strlen (ni->name) + 1
 			       + strlen (fct_name) + 1);
 	      char name[namlen];
 
 	      /* Construct the function name.  */
 	      __stpcpy (__stpcpy (__stpcpy (__stpcpy (name, "_nss_"),
-					    ni->library->name),
+					    ni->name),
 				  "_"),
 			fct_name);
 
@@ -457,12 +457,12 @@ __nss_lookup_function (service_user *ni, const char *fct_name)
 # include "function.def"
 		{ NULL, NULL }
 	      };
-	    size_t namlen = (5 + strlen (ni->library->name) + 1
+	    size_t namlen = (5 + strlen (ni->name) + 1
 			     + strlen (fct_name) + 1);
 	    char name[namlen];
 
 	    /* Construct the function name.  */
-	    __stpcpy (__stpcpy (__stpcpy (name, ni->library->name),
+	    __stpcpy (__stpcpy (__stpcpy (name, ni->name),
 				"_"),
 		      fct_name);
 
diff --git a/nss/tst-nss-static.c b/nss/tst-nss-static.c
new file mode 100644
index 0000000..98cf073
--- /dev/null
+++ b/nss/tst-nss-static.c
@@ -0,0 +1,15 @@
+/* glibc test for static NSS.  */
+#include <stdio.h>
+
+#define TEST_FUNCTION do_test ()
+static int
+do_test (void)
+{
+  struct passwd *pw;
+
+  pw = getpwuid(0);
+  return pw == NULL;
+}
+
+
+#include "../test-skeleton.c"