* [RFC] Fix netgroups handling
@ 2005-09-09 15:18 Jakub Jelinek
2005-09-10 3:25 ` Ulrich Drepper
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2005-09-09 15:18 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: Glibc hackers
Hi!
I believe there are several problems in netgroups handling
(setnetgrent/getnetgrent*/endnetgrent/innetgr).
1) data/data_size/cursor/position __netgrent fields are private to the
service. From what I understand, there needs to be always at most one service
live for each __netgrent object. But we should never allow for one
service to cleanup after a different service, as it doesn't understand
how it uses the fields internally. Say the call internal_endnetgrent
in NIS or NIS+'s setgrent hook if the fields were previously allocated
by NIS+ resp. files/NIS will do bad things.
2) using setup/__nss_lookup/__nss_next for hooks other than setgrent
seems to be wrong to me. These functions (might) modify __netgrent's
nip field, but that means that data populated by one service's
setnetgrent would be interpreted by a different service's
endnetgrent or getnetgrent_r hook
3) __internal_setnetgrent_reuse call in __internal_getnetgrent_r
(when some netgroup contains name of another netgroup) might be
provided by different service.
Say /etc/netgroup containing
foo (host1,user1,) bar
and say NIS+
bar (host2,user2,)
But in that case we need to look up a different function, reusing
the one from last service might be wrong.
4) if (result->cursor == NULL)
return NSS_STATUS_NOTFOUND;
in _nss_nis_getnetgrent_r is redundant IMHO, as _nss_netgroup_parseline
does the same check at the beginning.
The following (just compile tested) patch assumes that only
NSS_STATUS_SUCCESS returning setgrent hooks modify
data/data_size/cursor/position and in that case will call the endnetgrent
hook before switching to another service or another netgroup
and uses just __nss_lookup_function to look up endnetgrent/getnetgrent_r
hooks.
2005-09-09 Jakub Jelinek <jakub@redhat.com>
* inet/getnetgrent_r.c: Include assert.
(setup): Remove FUNC_NAME and ALL arguments, assume they are always
"setnetgrent" and 1.
(endnetgrent_hook): New function.
(internal_endnetgrent): Use it.
(__internal_setnetgrent_reuse): Use it. Adjust setup caller.
If status is NSS_STATUS_SUCCESS, yet action is continue, call
endnetgrent hook.
(internal_getnetgrent_r): Use __nss_lookup_function rather than
setup. Recompute getfct pointer after successful
__internal_setnetgrent_reuse. Don't use __nss_next.
(innetgr): Use __nss_lookup_function instead of __nss_lookup.
Adjust setup caller.
* nss/nss_files/files-netgrp.c (_nss_files_endnetgrent): Always clear
data_size and cursor. Add libnss_files_hidden_proto and
libnss_files_hidden_def.
(_nss_files_setnetgrent): Call _nss_files_endnetgrent on failure.
* nis/nss_nis/nis-netgrp.c (internal_endnetgrent): Always clear
data_size and cursor.
(_nss_nis_setnetgrent): Don't call internal_endnetgrent.
(_nss_nis_getnetgrent_r): Remove result->cursor == NULL handling.
* nis/nss_nisplus/nisplus-netgrp.c (internal_endnetgrent): Always clear
data_size and position.
(_nss_nisplus_setnetgrent): Don't call internal_endnetgrent.
--- libc/inet/getnetgrent_r.c.jj 2004-08-14 18:18:30.000000000 +0200
+++ libc/inet/getnetgrent_r.c 2005-09-09 15:22:13.000000000 +0200
@@ -1,4 +1,5 @@
-/* Copyright (C) 1996,1997,1998,1999,2002,2004 Free Software Foundation, Inc.
+/* Copyright (C) 1996, 1997, 1998, 1999, 2002, 2004, 2005
+ 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
@@ -16,6 +17,7 @@
Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA. */
+#include <assert.h>
#include <bits/libc-lock.h>
#include <errno.h>
#include <netdb.h>
@@ -33,15 +35,13 @@ __libc_lock_define_initialized (static,
static struct __netgrent dataset;
/* The lookup function for the first entry of this service. */
-extern int __nss_netgroup_lookup (service_user **nip, const char *name,
+extern int __nss_netgroup_lookup (service_user **nipp, const char *name,
void **fctp) internal_function;
-
-/* Set up NIP to run through the services. If ALL is zero, use NIP's
- current location if it's not nil. Return nonzero if there are no
+/* Set up NIP to run through the services. Return nonzero if there are no
services (left). */
static enum nss_status
-setup (void **fctp, const char *func_name, int all, service_user **nipp)
+setup (void **fctp, service_user **nipp)
{
/* Remember the first service_entry, it's always the same. */
static service_user *startp;
@@ -51,7 +51,7 @@ setup (void **fctp, const char *func_nam
{
/* Executing this more than once at the same time must yield the
same result every time. So we need no locking. */
- no_more = __nss_netgroup_lookup (nipp, func_name, fctp);
+ no_more = __nss_netgroup_lookup (nipp, "setnetgrent", fctp);
startp = no_more ? (service_user *) -1 : *nipp;
}
else if (startp == (service_user *) -1)
@@ -59,11 +59,10 @@ setup (void **fctp, const char *func_nam
return 1;
else
{
- if (all || *nipp == NULL)
- /* Reset to the beginning of the service list. */
- *nipp = startp;
+ /* Reset to the beginning of the service list. */
+ *nipp = startp;
/* Look up the first function. */
- no_more = __nss_lookup (nipp, func_name, fctp);
+ no_more = __nss_lookup (nipp, "setnetgrent", fctp);
}
return no_more;
}
@@ -87,6 +86,20 @@ free_memory (struct __netgrent *data)
}
}
\f
+static void
+endnetgrent_hook (struct __netgrent *datap)
+{
+ enum nss_status (*endfct) (struct __netgrent *);
+
+ if (datap->nip == NULL)
+ return;
+
+ endfct = __nss_lookup_function (datap->nip, "endnetgrent");
+ if (endfct != NULL)
+ (void) (*endfct) (datap);
+ datap->nip = NULL;
+}
+
static int
internal_function
__internal_setnetgrent_reuse (const char *group, struct __netgrent *datap,
@@ -100,14 +113,29 @@ __internal_setnetgrent_reuse (const char
enum nss_status status = NSS_STATUS_UNAVAIL;
struct name_list *new_elem;
+ /* Free data from previous service. */
+ endnetgrent_hook (datap);
+
/* Cycle through all the services and run their setnetgrent functions. */
- int no_more = setup (&fct.ptr, "setnetgrent", 1, &datap->nip);
+ int no_more = setup (&fct.ptr, &datap->nip);
while (! no_more)
{
+ assert (datap->data == NULL);
+
/* Ignore status, we force check in `__nss_next'. */
status = (*fct.f) (group, datap);
+ service_user *old_nip = datap->nip;
no_more = __nss_next (&datap->nip, "setnetgrent", &fct.ptr, status, 0);
+
+ if (status == NSS_STATUS_SUCCESS && ! no_more)
+ {
+ enum nss_status (*endfct) (struct __netgrent *);
+
+ endfct = __nss_lookup_function (old_nip, "endnetgrent");
+ if (endfct != NULL)
+ (void) (*endfct) (datap);
+ }
}
/* Add the current group to the list of known groups. */
@@ -157,34 +185,13 @@ setnetgrent (const char *group)
return result;
}
-
void internal_endnetgrent (struct __netgrent *datap);
libc_hidden_proto (internal_endnetgrent)
void
internal_endnetgrent (struct __netgrent *datap)
{
- service_user *old_nip;
- union
- {
- enum nss_status (*f) (struct __netgrent *);
- void *ptr;
- } fct;
-
- /* Remember which was the last used service. */
- old_nip = datap->nip;
-
- /* Cycle through all the services and run their endnetgrent functions. */
- int no_more = setup (&fct.ptr, "endnetgrent", 1, &datap->nip);
- while (! no_more)
- {
- /* Ignore status, we force check in `__nss_next'. */
- (void) (*fct.f) (datap);
-
- no_more = (datap->nip == old_nip
- || __nss_next (&datap->nip, "endnetgrent", &fct.ptr, 0, 1));
- }
-
+ endnetgrent_hook (datap);
/* Now free list of all netgroup names from last run. */
free_memory (datap);
}
@@ -213,11 +220,7 @@ internal_getnetgrent_r (char **hostp, ch
struct __netgrent *datap,
char *buffer, size_t buflen, int *errnop)
{
- union
- {
- enum nss_status (*f) (struct __netgrent *, char *, size_t, int *);
- void *ptr;
- } fct;
+ enum nss_status (*fct) (struct __netgrent *, char *, size_t, int *);
/* Initialize status to return if no more functions are found. */
enum nss_status status = NSS_STATUS_NOTFOUND;
@@ -225,10 +228,12 @@ internal_getnetgrent_r (char **hostp, ch
/* Run through available functions, starting with the same function last
run. We will repeat each function as long as it succeeds, and then go
on to the next service action. */
- int no_more = setup (&fct.ptr, "getnetgrent_r", 0, &datap->nip);
+ int no_more = (datap->nip == NULL
+ || (fct = __nss_lookup_function (datap->nip, "getnetgrent_r"))
+ == NULL);
while (! no_more)
{
- status = (*fct.f) (datap, buffer, buflen, &errno);
+ status = (*fct) (datap, buffer, buflen, &errno);
if (status == NSS_STATUS_RETURN)
{
@@ -246,8 +251,12 @@ internal_getnetgrent_r (char **hostp, ch
datap, errnop);
}
- if (found)
- continue;
+ if (found && datap->nip != NULL)
+ {
+ fct = __nss_lookup_function (datap->nip, "getnetgrent_r");
+ if (fct != NULL)
+ continue;
+ }
}
else if (status == NSS_STATUS_SUCCESS && datap->type == group_val)
{
@@ -279,7 +288,7 @@ internal_getnetgrent_r (char **hostp, ch
}
}
- no_more = __nss_next (&datap->nip, "getnetgrent_r", &fct.ptr, status, 0);
+ break;
}
if (status == NSS_STATUS_SUCCESS)
@@ -322,16 +331,8 @@ innetgr (const char *netgroup, const cha
int (*f) (const char *, struct __netgrent *);
void *ptr;
} setfct;
- union
- {
- void (*f) (struct __netgrent *);
- void *ptr;
- } endfct;
- union
- {
- int (*f) (struct __netgrent *, char *, size_t, int *);
- void *ptr;
- } getfct;
+ void (*endfct) (struct __netgrent *);
+ int (*getfct) (struct __netgrent *, char *, size_t, int *);
struct __netgrent entry;
int result = 0;
const char *current_group = netgroup;
@@ -345,18 +346,21 @@ innetgr (const char *netgroup, const cha
the work during one walk through the service list. */
while (1)
{
- int no_more = setup (&setfct.ptr, "setnetgrent", 1, &entry.nip);
+ int no_more = setup (&setfct.ptr, &entry.nip);
while (! no_more)
{
+ assert (entry.data == NULL);
+
/* Open netgroup. */
enum nss_status status = (*setfct.f) (current_group, &entry);
if (status == NSS_STATUS_SUCCESS
- && __nss_lookup (&entry.nip, "getnetgrent_r", &getfct.ptr) == 0)
+ && (getfct = __nss_lookup_function (entry.nip, "getnetgrent_r"))
+ != NULL)
{
char buffer[1024];
- while ((*getfct.f) (&entry, buffer, sizeof buffer, &errno)
+ while ((*getfct) (&entry, buffer, sizeof buffer, &errno)
== NSS_STATUS_SUCCESS)
{
if (entry.type == group_val)
@@ -414,8 +418,9 @@ innetgr (const char *netgroup, const cha
}
/* Free all resources of the service. */
- if (__nss_lookup (&entry.nip, "endnetgrent", &endfct.ptr) == 0)
- (*endfct.f) (&entry);
+ endfct = __nss_lookup_function (entry.nip, "endnetgrent");
+ if (endfct != NULL)
+ (*endfct) (&entry);
/* Look for the next service. */
no_more = __nss_next (&entry.nip, "setnetgrent",
--- libc/nis/nss_nisplus/nisplus-netgrp.c.jj 2004-12-13 09:36:17.000000000 +0100
+++ libc/nis/nss_nisplus/nisplus-netgrp.c 2005-09-09 13:42:45.000000000 +0200
@@ -1,4 +1,4 @@
-/* Copyright (C) 1997, 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 1997, 2003, 2004, 2005 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Thorsten Kukuk <kukuk@vt.uni-paderborn.de>, 1997.
@@ -142,12 +142,10 @@ static void
internal_endnetgrent (struct __netgrent *netgrp)
{
if (netgrp->data != NULL)
- {
- nis_freeresult ((nis_result *) netgrp->data);
- netgrp->data = NULL;
- netgrp->data_size = 0;
- netgrp->position = 0;
- }
+ nis_freeresult ((nis_result *) netgrp->data);
+ netgrp->data = NULL;
+ netgrp->data_size = 0;
+ netgrp->position = 0;
}
enum nss_status
@@ -161,8 +159,6 @@ _nss_nisplus_setnetgrent (const char *gr
status = NSS_STATUS_SUCCESS;
- internal_endnetgrent (netgrp);
-
sprintf (buf, "[name=%s],netgroup.org_dir", group);
netgrp->data = (char *) nis_list (buf, EXPAND_NAME, NULL, NULL);
--- libc/nis/nss_nis/nis-netgrp.c.jj 2005-08-23 12:00:37.000000000 +0200
+++ libc/nis/nss_nis/nis-netgrp.c 2005-09-09 13:40:34.000000000 +0200
@@ -41,13 +41,10 @@ _nss_netgroup_parseline (char **cursor,
static void
internal_nis_endnetgrent (struct __netgrent *netgrp)
{
- if (netgrp->data != NULL)
- {
- free (netgrp->data);
- netgrp->data = NULL;
- netgrp->data_size = 0;
- netgrp->cursor = NULL;
- }
+ free (netgrp->data);
+ netgrp->data = NULL;
+ netgrp->data_size = 0;
+ netgrp->cursor = NULL;
}
enum nss_status
@@ -65,8 +62,6 @@ _nss_nis_setnetgrent (const char *group,
if (yp_get_default_domain (&domain))
return NSS_STATUS_UNAVAIL;
- internal_nis_endnetgrent (netgrp);
-
status = yperr2nss (yp_match (domain, "netgroup", group, strlen (group),
&netgrp->data, &len));
if (status == NSS_STATUS_SUCCESS)
@@ -99,9 +94,6 @@ enum nss_status
_nss_nis_getnetgrent_r (struct __netgrent *result, char *buffer, size_t buflen,
int *errnop)
{
- if (result->cursor == NULL)
- return NSS_STATUS_NOTFOUND;
-
return _nss_netgroup_parseline (&result->cursor, result, buffer, buflen,
errnop);
}
--- libc/nss/nss_files/files-netgrp.c.jj 2004-11-23 21:42:29.000000000 +0100
+++ libc/nss/nss_files/files-netgrp.c 2005-09-09 13:35:31.000000000 +0200
@@ -1,5 +1,5 @@
/* Netgroup file parser in nss_files modules.
- Copyright (C) 1996, 1997, 2000, 2004 Free Software Foundation, Inc.
+ Copyright (C) 1996, 1997, 2000, 2004, 2005 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@cygnus.com>, 1996.
@@ -29,6 +29,7 @@
#define DATAFILE "/etc/netgroup"
+libnss_files_hidden_proto (_nss_files_endnetgrent)
#define EXPAND(needed) \
do \
@@ -140,6 +141,9 @@ _nss_files_setnetgrent (const char *grou
/* We don't need the file and the line buffer anymore. */
free (line);
fclose (fp);
+
+ if (status != NSS_STATUS_SUCCESS)
+ _nss_files_endnetgrent (result);
}
return status;
@@ -150,16 +154,13 @@ int
_nss_files_endnetgrent (struct __netgrent *result)
{
/* Free allocated memory for data if some is present. */
- if (result->data != NULL)
- {
- free (result->data);
- result->data = NULL;
- result->data_size = 0;
- result->cursor = NULL;
- }
-
+ free (result->data);
+ result->data = NULL;
+ result->data_size = 0;
+ result->cursor = NULL;
return NSS_STATUS_SUCCESS;
}
+libnss_files_hidden_def (_nss_files_endnetgrent)
static char *
strip_whitespace (char *str)
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [RFC] Fix netgroups handling
2005-09-09 15:18 [RFC] Fix netgroups handling Jakub Jelinek
@ 2005-09-10 3:25 ` Ulrich Drepper
0 siblings, 0 replies; 2+ messages in thread
From: Ulrich Drepper @ 2005-09-10 3:25 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Glibc hackers
[-- Attachment #1: Type: text/plain, Size: 167 bytes --]
I think the patch is fine. I made a few minor changes and ran some tests.
--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2005-09-10 3:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-09-09 15:18 [RFC] Fix netgroups handling Jakub Jelinek
2005-09-10 3:25 ` Ulrich Drepper
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).