| Message ID | xnczzib9pw.fsf@greed.delorie.com |
|---|---|
| State | New |
| Headers | show |
| Series | nsswitch: handle missing actions properly | expand |
On 12/10/20 9:20 AM, DJ Delorie via Libc-alpha wrote: > > RCA: the __nss_database_get return value is nonzero on ERROR, not on > MISSING. A separate check for MISSING is needed. This only really > affects initgroups, since it has a fallback, so needs to know if > initgroups is missing from nsswitch.conf. Note: it's now possible to > have a line in nsswitch.conf like this: > > initgroups: > > That is *not* MISSING but has an empty module list. If this is > undesired behavior, a further "&& nip->module" is needed. The > nss_database.c patch ensures an empty module list for an empty > nsswitch.conf list, instead of > > See also https://bugzilla.redhat.com/show_bug.cgi?id=1906066 > > Actual proposed commit follows: > > ----- > Some internal functions need to know if a database has a nonzero > list of actions; success getting the database does not guarantee > that. Add checks for such as needed. > > Skip the ":" in each nsswitch.conf line so as not to add a dummy > action libnss_:.so The fix is OK but there ought to be a regression test that verifies that getgroups() returns the full list of supplementary groups. I'm not sure if the container testing can handle this though. Siddhesh
* Siddhesh Poyarekar: > The fix is OK but there ought to be a regression test that verifies > that getgroups() returns the full list of supplementary groups. I'm > not sure if the container testing can handle this though. We can test getgrouplist, which does not change the supplementary groups, but exercises the same NSS code in the background (via internal_getgrouplist that is also called by initgroups). Thanks, Florian
On Dez 09 2020, DJ Delorie via Libc-alpha wrote: > diff --git a/grp/initgroups.c b/grp/initgroups.c > index a60ca1c395..a0a836d862 100644 > --- a/grp/initgroups.c > +++ b/grp/initgroups.c > @@ -72,11 +72,13 @@ internal_getgrouplist (const char *user, gid_t group, long int *size, > > nss_action_list nip; > > - if (__nss_database_get (nss_database_initgroups, &nip)) > + if (__nss_database_get (nss_database_initgroups, &nip) > + && nip != NULL) > { > use_initgroups_entry = true; > } > - else if (__nss_database_get (nss_database_group, &nip)) > + else if (__nss_database_get (nss_database_group, &nip) > + && nip != NULL) Indentation is off here. > diff --git a/nss/nsswitch.c b/nss/nsswitch.c > index 40109c744d..921062e04f 100644 > --- a/nss/nsswitch.c > +++ b/nss/nsswitch.c > @@ -81,7 +81,7 @@ __nss_database_lookup2 (const char *database, const char *alternate_name, > if (database_names[database_id] == NULL) > return -1; > > - if (__nss_database_get (database_id, ni)) > + if (__nss_database_get (database_id, ni) && *ni) No implicit boolean coercion. Andreas.
diff --git a/grp/initgroups.c b/grp/initgroups.c index a60ca1c395..a0a836d862 100644 --- a/grp/initgroups.c +++ b/grp/initgroups.c @@ -72,11 +72,13 @@ internal_getgrouplist (const char *user, gid_t group, long int *size, nss_action_list nip; - if (__nss_database_get (nss_database_initgroups, &nip)) + if (__nss_database_get (nss_database_initgroups, &nip) + && nip != NULL) { use_initgroups_entry = true; } - else if (__nss_database_get (nss_database_group, &nip)) + else if (__nss_database_get (nss_database_group, &nip) + && nip != NULL) { use_initgroups_entry = false; } diff --git a/nss/nss_database.c b/nss/nss_database.c index e8c307d1f3..a036e95fbf 100644 --- a/nss/nss_database.c +++ b/nss/nss_database.c @@ -212,7 +212,8 @@ process_line (struct nss_database_data *data, char *line) if (line[0] == '\0' || name == line) /* Syntax error. Skip this line. */ return true; - *line++ = '\0'; + while (line[0] != '\0' && (isspace (line[0]) || line[0] == ':')) + *line++ = '\0'; int db = name_to_database_index (name); if (db < 0) diff --git a/nss/nsswitch.c b/nss/nsswitch.c index 40109c744d..921062e04f 100644 --- a/nss/nsswitch.c +++ b/nss/nsswitch.c @@ -81,7 +81,7 @@ __nss_database_lookup2 (const char *database, const char *alternate_name, if (database_names[database_id] == NULL) return -1; - if (__nss_database_get (database_id, ni)) + if (__nss_database_get (database_id, ni) && *ni) { /* Success. */ return 0;