nss: fix nss_database_lookup2's alternate handling [27416]

Message ID xn4ki77l4v.fsf@greed.delorie.com
State Superseded
Headers show
Series
  • nss: fix nss_database_lookup2's alternate handling [27416]
Related show

Commit Message

DJ Delorie via Libc-alpha Feb. 19, 2021, 10:22 p.m.
https://sourceware.org/bugzilla/show_bug.cgi?id=27416

Comments

DJ Delorie via Libc-alpha Feb. 20, 2021, 4:19 p.m. | #1
On 2/19/21 5:22 PM, DJ Delorie via Libc-alpha wrote:
> 

> https://sourceware.org/bugzilla/show_bug.cgi?id=27416


Could you please provide the full commit message you plan to
use for this commit?

If you commit your change locally then do:
git format-patch HEAD~1
git send-email mydiff.patch

That will do everything you need automatically. It avoids
having to anything manually like generating a subject like
with lowercase 'patch' or failing to include the exact commit
message as it will be when you apply the patch.

Please repost v2.

> diff --git a/nss/Makefile b/nss/Makefile

> index 0906202db9..71fbe583bf 100644

> --- a/nss/Makefile

> +++ b/nss/Makefile

> @@ -63,6 +63,7 @@ tests			= test-netdb test-digits-dots tst-nss-getpwent bug17079 \

>  xtests			= bug-erange

>  

>  tests-container = \

> +			  tst-nss-compat1 \


OK. New container test.

>  			  tst-nss-test3 \

>  			  tst-nss-files-hosts-long \

>  			  tst-nss-db-endpwent \

> diff --git a/nss/databases.def b/nss/databases.def

> index df5fab4168..3dc95648a8 100644

> --- a/nss/databases.def

> +++ b/nss/databases.def

> @@ -23,17 +23,20 @@

>  DEFINE_DATABASE (aliases)

>  DEFINE_DATABASE (ethers)

>  DEFINE_DATABASE (group)

> +DEFINE_DATABASE (group_compat)

>  DEFINE_DATABASE (gshadow)

>  DEFINE_DATABASE (hosts)

>  DEFINE_DATABASE (initgroups)

>  DEFINE_DATABASE (netgroup)

>  DEFINE_DATABASE (networks)

>  DEFINE_DATABASE (passwd)

> +DEFINE_DATABASE (passwd_compat)

>  DEFINE_DATABASE (protocols)

>  DEFINE_DATABASE (publickey)

>  DEFINE_DATABASE (rpc)

>  DEFINE_DATABASE (services)

>  DEFINE_DATABASE (shadow)

> +DEFINE_DATABASE (shadow_compat)


OK. Define the three compat databases for use in the generic code.

>  

>  /*

>     Local Variables:

> diff --git a/nss/nss_database.c b/nss/nss_database.c

> index fb72d0cc03..9ff3bb6ffb 100644

> --- a/nss/nss_database.c

> +++ b/nss/nss_database.c

> @@ -172,7 +172,7 @@ nss_database_select_default (struct nss_database_default_cache *cache,

>  

>  /* database_name must be large enough for each individual name plus a

>     null terminator.  */

> -typedef char database_name[11];

> +typedef char database_name[14];


OK. Increase array size.

>  #define DEFINE_DATABASE(name) \

>    _Static_assert (sizeof (#name) <= sizeof (database_name), #name);

>  #include "databases.def"

> diff --git a/nss/nsswitch.c b/nss/nsswitch.c

> index 46f232d720..6d8673b507 100644

> --- a/nss/nsswitch.c

> +++ b/nss/nsswitch.c

> @@ -76,24 +76,26 @@ __nss_database_lookup2 (const char *database, const char *alternate_name,

>  

>    for (database_id = 0; database_names[database_id]; database_id++)

>      if (strcmp (database_names[database_id], database) == 0)

> -	break;

> -

> -  if (database_names[database_id] == NULL)

> -    return -1;

> -

> -  /* If *NI is NULL, the database was not mentioned in nsswitch.conf.

> -     If *NI is not NULL, but *NI->module is NULL, the database was in

> -     nsswitch.conf but listed no actions.  We test for the former.  */

> -  if (__nss_database_get (database_id, ni) && *ni != NULL)

> -    {

> -      /* Success.  */

> -      return 0;

> -    }

> -  else

> -    {

> -      /* Failure.  */

> -      return -1;

> -    }

> +      /* If *NI is NULL, the database was not mentioned in nsswitch.conf.

> +	 If *NI is not NULL, but *NI->module is NULL, the database was in

> +	 nsswitch.conf but listed no actions.  We test for the former.  */

> +      if (__nss_database_get (database_id, ni) && *ni != NULL)

> +	return 0;

> +

> +  /* Primary name not found, try alternate.  */

> +  if (alternate_name)

> +    for (database_id = 0; database_names[database_id]; database_id++)

> +      if (strcmp (database_names[database_id], alternate_name) == 0)

> +	if (__nss_database_get (database_id, ni) && *ni != NULL)

> +	  return 0;

> +

> +  /* Neither found, use default config.  */

> +  *ni = __nss_action_parse (defconfig);

> +  if (*ni != NULL)

> +    return 0;

> +

> +  /* Failure.  */

> +  return -1;


OK. Add code to try the alternate or default.

>  }

>  libc_hidden_def (__nss_database_lookup2)

>  

> diff --git a/nss/tst-nss-compat1.c b/nss/tst-nss-compat1.c

> new file mode 100644

> index 0000000000..45355ef225

> --- /dev/null

> +++ b/nss/tst-nss-compat1.c

> @@ -0,0 +1,64 @@

> +/* Test error checking for group entries.

> +   Copyright (C) 2021 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

> +   <https://www.gnu.org/licenses/>.  */

> +

> +#include <nss.h>

> +#include <stdio.h>

> +#include <stdlib.h>

> +#include <string.h>

> +

> +#include <support/support.h>

> +#include <support/check.h>

> +

> +#include "nss_test.h"

> +

> +static struct passwd pwd_table[] = {

> +    PWD (100),

> +    PWD (30),

> +    PWD_LAST ()

> +  };

> +

> +void

> +_nss_test1_init_hook(test_tables *t)

> +{

> +  t->pwd_table = pwd_table;

> +}

> +

> +static int

> +do_test (void)

> +{

> +  struct passwd *p = NULL;

> +  struct group *g = NULL;

> +

> +  /* Test that compat-to-test works.  */

> +  p = getpwuid (100);

> +  if (p == NULL)

> +    FAIL_EXIT1("getpwuid-compat-test1 p");

> +  else if (strcmp (p->pw_name, "name100") != 0)

> +    FAIL_EXIT1("getpwuid-compat-test1 name100");


OK. Password uses compat which redirects to test1 instead of files.
So seeing name100 means files was used and that's wrong.

> +

> +  /* Test that internal defconfig works.  */

> +  g = getgrgid (100);

> +  if (g == NULL)

> +    FAIL_EXIT1("getgrgid-compat-null");

> +  if (strcmp (g->gr_name, "wilma") != 0)

> +    FAIL_EXIT1("getgrgid-compat-name");


OK.

> +

> +  return 0;

> +}

> +

> +#include <support/test-driver.c>

> diff --git a/nss/tst-nss-compat1.root/etc/group b/nss/tst-nss-compat1.root/etc/group

> new file mode 100644

> index 0000000000..ee467c7950

> --- /dev/null

> +++ b/nss/tst-nss-compat1.root/etc/group

> @@ -0,0 +1 @@

> +wilma:x:100:

> diff --git a/nss/tst-nss-compat1.root/etc/nsswitch.conf b/nss/tst-nss-compat1.root/etc/nsswitch.conf

> new file mode 100644

> index 0000000000..7fe69d5ffa

> --- /dev/null

> +++ b/nss/tst-nss-compat1.root/etc/nsswitch.conf

> @@ -0,0 +1,3 @@

> +passwd : compat

> +passwd_compat : test1

> +

> diff --git a/nss/tst-nss-compat1.root/etc/passwd b/nss/tst-nss-compat1.root/etc/passwd

> new file mode 100644

> index 0000000000..84635587bd

> --- /dev/null

> +++ b/nss/tst-nss-compat1.root/etc/passwd

> @@ -0,0 +1,3 @@

> +name5:x:5:555:name5 for testing:/home/name5:/bin/nologin

> ++name100

> ++name30

> diff --git a/nss/tst-nss-compat1.root/tst-nss-compat1.script b/nss/tst-nss-compat1.root/tst-nss-compat1.script

> new file mode 100644

> index 0000000000..fe6e863f01

> --- /dev/null

> +++ b/nss/tst-nss-compat1.root/tst-nss-compat1.script

> @@ -0,0 +1 @@

> +cp $B/nss/libnss_test1.so $L/libnss_test1.so.2

> 



-- 
Cheers,
Carlos.
Andreas Schwab Feb. 22, 2021, 12:13 p.m. | #2
Still broken.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Patch

diff --git a/nss/Makefile b/nss/Makefile
index 0906202db9..71fbe583bf 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -63,6 +63,7 @@  tests			= test-netdb test-digits-dots tst-nss-getpwent bug17079 \
 xtests			= bug-erange
 
 tests-container = \
+			  tst-nss-compat1 \
 			  tst-nss-test3 \
 			  tst-nss-files-hosts-long \
 			  tst-nss-db-endpwent \
diff --git a/nss/databases.def b/nss/databases.def
index df5fab4168..3dc95648a8 100644
--- a/nss/databases.def
+++ b/nss/databases.def
@@ -23,17 +23,20 @@ 
 DEFINE_DATABASE (aliases)
 DEFINE_DATABASE (ethers)
 DEFINE_DATABASE (group)
+DEFINE_DATABASE (group_compat)
 DEFINE_DATABASE (gshadow)
 DEFINE_DATABASE (hosts)
 DEFINE_DATABASE (initgroups)
 DEFINE_DATABASE (netgroup)
 DEFINE_DATABASE (networks)
 DEFINE_DATABASE (passwd)
+DEFINE_DATABASE (passwd_compat)
 DEFINE_DATABASE (protocols)
 DEFINE_DATABASE (publickey)
 DEFINE_DATABASE (rpc)
 DEFINE_DATABASE (services)
 DEFINE_DATABASE (shadow)
+DEFINE_DATABASE (shadow_compat)
 
 /*
    Local Variables:
diff --git a/nss/nss_database.c b/nss/nss_database.c
index fb72d0cc03..9ff3bb6ffb 100644
--- a/nss/nss_database.c
+++ b/nss/nss_database.c
@@ -172,7 +172,7 @@  nss_database_select_default (struct nss_database_default_cache *cache,
 
 /* database_name must be large enough for each individual name plus a
    null terminator.  */
-typedef char database_name[11];
+typedef char database_name[14];
 #define DEFINE_DATABASE(name) \
   _Static_assert (sizeof (#name) <= sizeof (database_name), #name);
 #include "databases.def"
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 46f232d720..6d8673b507 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -76,24 +76,26 @@  __nss_database_lookup2 (const char *database, const char *alternate_name,
 
   for (database_id = 0; database_names[database_id]; database_id++)
     if (strcmp (database_names[database_id], database) == 0)
-	break;
-
-  if (database_names[database_id] == NULL)
-    return -1;
-
-  /* If *NI is NULL, the database was not mentioned in nsswitch.conf.
-     If *NI is not NULL, but *NI->module is NULL, the database was in
-     nsswitch.conf but listed no actions.  We test for the former.  */
-  if (__nss_database_get (database_id, ni) && *ni != NULL)
-    {
-      /* Success.  */
-      return 0;
-    }
-  else
-    {
-      /* Failure.  */
-      return -1;
-    }
+      /* If *NI is NULL, the database was not mentioned in nsswitch.conf.
+	 If *NI is not NULL, but *NI->module is NULL, the database was in
+	 nsswitch.conf but listed no actions.  We test for the former.  */
+      if (__nss_database_get (database_id, ni) && *ni != NULL)
+	return 0;
+
+  /* Primary name not found, try alternate.  */
+  if (alternate_name)
+    for (database_id = 0; database_names[database_id]; database_id++)
+      if (strcmp (database_names[database_id], alternate_name) == 0)
+	if (__nss_database_get (database_id, ni) && *ni != NULL)
+	  return 0;
+
+  /* Neither found, use default config.  */
+  *ni = __nss_action_parse (defconfig);
+  if (*ni != NULL)
+    return 0;
+
+  /* Failure.  */
+  return -1;
 }
 libc_hidden_def (__nss_database_lookup2)
 
diff --git a/nss/tst-nss-compat1.c b/nss/tst-nss-compat1.c
new file mode 100644
index 0000000000..45355ef225
--- /dev/null
+++ b/nss/tst-nss-compat1.c
@@ -0,0 +1,64 @@ 
+/* Test error checking for group entries.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <nss.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <support/support.h>
+#include <support/check.h>
+
+#include "nss_test.h"
+
+static struct passwd pwd_table[] = {
+    PWD (100),
+    PWD (30),
+    PWD_LAST ()
+  };
+
+void
+_nss_test1_init_hook(test_tables *t)
+{
+  t->pwd_table = pwd_table;
+}
+
+static int
+do_test (void)
+{
+  struct passwd *p = NULL;
+  struct group *g = NULL;
+
+  /* Test that compat-to-test works.  */
+  p = getpwuid (100);
+  if (p == NULL)
+    FAIL_EXIT1("getpwuid-compat-test1 p");
+  else if (strcmp (p->pw_name, "name100") != 0)
+    FAIL_EXIT1("getpwuid-compat-test1 name100");
+
+  /* Test that internal defconfig works.  */
+  g = getgrgid (100);
+  if (g == NULL)
+    FAIL_EXIT1("getgrgid-compat-null");
+  if (strcmp (g->gr_name, "wilma") != 0)
+    FAIL_EXIT1("getgrgid-compat-name");
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/nss/tst-nss-compat1.root/etc/group b/nss/tst-nss-compat1.root/etc/group
new file mode 100644
index 0000000000..ee467c7950
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/group
@@ -0,0 +1 @@ 
+wilma:x:100:
diff --git a/nss/tst-nss-compat1.root/etc/nsswitch.conf b/nss/tst-nss-compat1.root/etc/nsswitch.conf
new file mode 100644
index 0000000000..7fe69d5ffa
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/nsswitch.conf
@@ -0,0 +1,3 @@ 
+passwd : compat
+passwd_compat : test1
+
diff --git a/nss/tst-nss-compat1.root/etc/passwd b/nss/tst-nss-compat1.root/etc/passwd
new file mode 100644
index 0000000000..84635587bd
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/passwd
@@ -0,0 +1,3 @@ 
+name5:x:5:555:name5 for testing:/home/name5:/bin/nologin
++name100
++name30
diff --git a/nss/tst-nss-compat1.root/tst-nss-compat1.script b/nss/tst-nss-compat1.root/tst-nss-compat1.script
new file mode 100644
index 0000000000..fe6e863f01
--- /dev/null
+++ b/nss/tst-nss-compat1.root/tst-nss-compat1.script
@@ -0,0 +1 @@ 
+cp $B/nss/libnss_test1.so $L/libnss_test1.so.2