* [patch] nss: fix nss_database_lookup2's alternate handling [27416]
@ 2021-02-19 22:22 DJ Delorie
2021-02-20 16:19 ` Carlos O'Donell
2021-02-22 12:13 ` Andreas Schwab
0 siblings, 2 replies; 5+ messages in thread
From: DJ Delorie @ 2021-02-19 22:22 UTC (permalink / raw)
To: libc-alpha
https://sourceware.org/bugzilla/show_bug.cgi?id=27416
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] nss: fix nss_database_lookup2's alternate handling [27416]
2021-02-19 22:22 [patch] nss: fix nss_database_lookup2's alternate handling [27416] DJ Delorie
@ 2021-02-20 16:19 ` Carlos O'Donell
2021-02-23 1:16 ` DJ Delorie
2021-02-22 12:13 ` Andreas Schwab
1 sibling, 1 reply; 5+ messages in thread
From: Carlos O'Donell @ 2021-02-20 16:19 UTC (permalink / raw)
To: DJ Delorie, libc-alpha
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] nss: fix nss_database_lookup2's alternate handling [27416]
2021-02-19 22:22 [patch] nss: fix nss_database_lookup2's alternate handling [27416] DJ Delorie
2021-02-20 16:19 ` Carlos O'Donell
@ 2021-02-22 12:13 ` Andreas Schwab
1 sibling, 0 replies; 5+ messages in thread
From: Andreas Schwab @ 2021-02-22 12:13 UTC (permalink / raw)
To: DJ Delorie via Libc-alpha
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."
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] nss: fix nss_database_lookup2's alternate handling [27416]
2021-02-20 16:19 ` Carlos O'Donell
@ 2021-02-23 1:16 ` DJ Delorie
2021-02-23 4:06 ` Carlos O'Donell
0 siblings, 1 reply; 5+ messages in thread
From: DJ Delorie @ 2021-02-23 1:16 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: libc-alpha
"Carlos O'Donell" <carlos@redhat.com> writes:
> 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?
That was it. Is more required?
> If you commit your change locally then do:
> git format-patch HEAD~1
> git send-email mydiff.patch
I commit my change locally, use gitk to extract it, then mail it on a
different machine.
> Please repost v2.
What changes are required?
>> + 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.
"name100" comes from test1; we're testing that it *is* returned, which
means test1 *is* being used to back compat.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] nss: fix nss_database_lookup2's alternate handling [27416]
2021-02-23 1:16 ` DJ Delorie
@ 2021-02-23 4:06 ` Carlos O'Donell
0 siblings, 0 replies; 5+ messages in thread
From: Carlos O'Donell @ 2021-02-23 4:06 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha
On 2/22/21 8:16 PM, DJ Delorie wrote:
> "Carlos O'Donell" <carlos@redhat.com> writes:
>> 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?
>
> That was it. Is more required?
The commit message should allow a reviewer to understand the
problem in the briefest form without external reference. A few
sentences about how we got here and why, and what corrective
steps are being taken would be useful for a commit message.
For inspiration please see:
https://sourceware.org/glibc/wiki/Contribution%20checklist#Commit_message_body
A full URL reference to sourceware is also not required.
The subject bug reference should be in one of two forms for
automated processing: (bug xxxx), or [BZ #xxxx]. Please adjust
>> If you commit your change locally then do:
>> git format-patch HEAD~1
>> git send-email mydiff.patch
>
> I commit my change locally, use gitk to extract it, then mail it on a
> different machine.
That's all OK, you can run git send-email on that remote system if you like
(avoids MUA problems).
https://sourceware.org/glibc/wiki/Contribution%20checklist#Generate_patch
`git format-patch` is the recommended way to get the format that a reviewer
is expecting.
>> Please repost v2.
>
> What changes are required?
Please repost v2 with the suggested commit message changes for review.
Reviewing the commit message is part of the review.
When the review is complete the only difference between what is reviewed
and what is committed should be the added Reviewed-by line.
>>> + 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.
>
> "name100" comes from test1; we're testing that it *is* returned, which
> means test1 *is* being used to back compat.
OK.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-23 4:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 22:22 [patch] nss: fix nss_database_lookup2's alternate handling [27416] DJ Delorie
2021-02-20 16:19 ` Carlos O'Donell
2021-02-23 1:16 ` DJ Delorie
2021-02-23 4:06 ` Carlos O'Donell
2021-02-22 12:13 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).