[PATCH] getgroups() gives wrong result with nss_winbind

Henrik Nordstrom hno at squid-cache.org
Sat Sep 18 16:38:36 GMT 2004


On Sat, 18 Sep 2004, Andreas wrote:

> I looked here:
> nis/nss_compat/compat-initgroups.c:418
>
>  tmpbuf = __alloca (buflen);
>
>  do
>    {
>      while ((status = internal_getgrent_r (&intern, tmpbuf, buflen,
>                                            user, group, start, size,
>                                            groupsp, limit, errnop))
>             == NSS_STATUS_TRYAGAIN && *errnop == ERANGE)
>        tmpbuf = extend_alloca (tmpbuf, buflen, 2 * buflen);
>    }
>  while (status == NSS_STATUS_SUCCESS);


Which isn't where you need to look as this is another buffer used for a 
similar but different purpose.

Where you need to look is the check_and_add_group function in the same 
file. Or if you like the winbind_nss_linit initgroups function.

This part of the NSS module interface is quite oddly done interface with 
conflicting documentation. To recap based on the source the actual 
initgroups NSS interface looks like

   NSS initgroups function is called with

      const char *user;	Login name
      gid_t group;	Primary group
      long int *start;	Number of group entries already populated
      gid_t **bufp;	Return of groups
      long int *size;	Initial size of the above return buffer, in number of entries.
      long int limit;	Maximum allowed number of returned entries, <= 0 no limit
      int *ettnop;	Errno return

And the NSS module is supposed to realloc *bufp while populating the 
entries until limit is reached, and in addition to prune out duplicate 
entries. Duplicate entries can happen when there is multiple NSS modules 
used mapping to the same gid. This is what the check_and_add_group 
function in the compat NSS module does, and also the nis module (except 
that it does not seem to prune duplicates).

The ERANGE errno is only valid for the getgrent type functions, to handle 
very large groups and similar situations. These functions are given a 
direct pointer to the buffer, not a pointer to a pointer.

Samba-3.0.7 even has code for this, but a few minor mistakes relating to 
the limit parameter makes it never trigger.. Attached is a (untested) 
patch which should address this and a few other bugs in the same code is 
attached.

Regards
Henrik
-------------- next part --------------
--- source/nsswitch/winbind_nss_linux.c.orig	2004-09-18 18:19:48.583877541 +0200
+++ source/nsswitch/winbind_nss_linux.c	2004-09-18 18:33:18.599320673 +0200
@@ -835,23 +835,35 @@
 
 			if (gid_list[i] == group) continue;
 
-			/* Add to buffer */
+			/* Filled buffer? */
 
-			if (*start == *size && limit <= 0) {
-				(*groups) = realloc(
-					(*groups), (2 * (*size) + 1) * sizeof(**groups));
-				if (! *groups) goto done;
-				*size = 2 * (*size) + 1;
+			if (*start == *size) {
+				long int newsize;
+				gid_t *newgroups;
+
+				newsize = 2 * *size;
+				if (limit > 0) {
+				    if (*size == limit)
+					goto done;
+				    newsize = MIN(newsize, limit);
+				}
+
+				newgroups = realloc(
+					(*groups), newsize * sizeof(**groups));
+				if (!newgroups) {
+				    *errnop = ENOMEM;
+				    ret = NSS_STATUS_NOTFOUND;
+				    goto done;
+				}
+				*groups = newgroups;
+				*size = newsize;
 			}
 
-			if (*start == *size) goto done;
+			/* Add to buffer */
 
 			(*groups)[*start] = gid_list[i];
 			*start += 1;
 
-			/* Filled buffer? */
-
-			if (*start == limit) goto done;
 		}
 	}
 	


More information about the samba-technical mailing list