From owner-freebsd-bugs@FreeBSD.ORG Fri May 5 22:40:16 2006 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0834316A408 for ; Fri, 5 May 2006 22:40:15 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 59B6E43D45 for ; Fri, 5 May 2006 22:40:15 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k45MeF6q012247 for ; Fri, 5 May 2006 22:40:15 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k45MeFIw012246; Fri, 5 May 2006 22:40:15 GMT (envelope-from gnats) Resent-Date: Fri, 5 May 2006 22:40:15 GMT Resent-Message-Id: <200605052240.k45MeFIw012246@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Kirk Webb Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 91ED016A401 for ; Fri, 5 May 2006 22:31:16 +0000 (UTC) (envelope-from kwebb@flux.utah.edu) Received: from bas.flux.utah.edu (bas.flux.utah.edu [155.98.60.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3B34E43D45 for ; Fri, 5 May 2006 22:31:16 +0000 (GMT) (envelope-from kwebb@flux.utah.edu) Received: from vodka.flux.utah.edu (vodka.flux.utah.edu [155.98.60.36]) by bas.flux.utah.edu (8.12.11/8.12.11) with ESMTP id k45MVF6m084617 for ; Fri, 5 May 2006 16:31:15 -0600 (MDT) (envelope-from kwebb@flux.utah.edu) Received: from vodka.flux.utah.edu (localhost [127.0.0.1]) by vodka.flux.utah.edu (8.13.4/8.13.4) with ESMTP id k45MVFqF058232 for ; Fri, 5 May 2006 16:31:15 -0600 (MDT) (envelope-from kwebb@vodka.flux.utah.edu) Received: (from kwebb@localhost) by vodka.flux.utah.edu (8.13.4/8.13.4/Submit) id k45MVFTB058231; Fri, 5 May 2006 16:31:15 -0600 (MDT) (envelope-from kwebb) Message-Id: <200605052231.k45MVFTB058231@vodka.flux.utah.edu> Date: Fri, 5 May 2006 16:31:15 -0600 (MDT) From: Kirk Webb To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: kern/96840: [patch] getgrent() does not return large groups via NIS X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Kirk Webb List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 May 2006 22:40:16 -0000 >Number: 96840 >Category: kern >Synopsis: [patch] getgrent() does not return large groups via NIS >Confidential: no >Severity: serious >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Fri May 05 22:40:14 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Kirk Webb >Release: FreeBSD 6.0-RELEASE-p6 i386 >Organization: University of Utah >Environment: System: FreeBSD vodka 6.0-RELEASE-p6 FreeBSD 6.0-RELEASE-p6 #0: Fri Apr 7 15:13:12 MDT 2006 root@vodka:/usr/obj/z/src/sys/DESKTOP i386 >Description: The nis_groups() function in src/lib/libc/gen/getgrent.c skips groups with a large number of members and/or large number of total characters in the membership list when fetching from NIS. Thus, anything using or vectoring through getgrent() (e.g. initgroups() and getgrouplist()) will not see these groups. getgrent_r is also affected, although the caller may pass in a larger buffer and so avoid the problem. The most obviously problematic side-effect of this behavior is that users end up with groups missing from their groups list, and so have reduced/incorrect permissions. >How-To-Repeat: Add something like the following group line to an NIS server with a FreeBSD client, with a legitimate user placed somewhere in the list (just make sure the member list is long): footest:*:6666:f1,f2,f3,f4,f5,f6,f7,f8,f9,f10,f11,f12,f13,f14,f15,f16,f17,f18,f19,f20,f21,f22,f23,f24,f25,f26,f27,f28,f29,f30,f31,f32,f33,f34,f35,f36,f37,f38,f39,f40,f41,f42,f43,f44,f45,f46,f47,f48,f49,f50,f51,f52,f53,f54,f55,f56,f57,f58,f59,f60,f61,f62,f63,f64,f65,f66,f67,f68,f69,f70,f71,f72,f73,f74,f75,f76,f77,f78,f79,f80,f81,f82,f83,f84,f85,f86,f87,f88,f89,f90,f91,f92,f93,f94,f95,f96,f97,f98,f99,f100,f101,f102,f103,f104,f105,f106,f107,f108,f109,f110,f111,f112,f113,f114,f115,f116,f117,f118,f119,f120,f121,f122,f123,f124,f125,f126,f127,f128,f129 Next, go to a FreeBSD 6 client bound to this server and execute 'groups' or 'id' for the user. The test list should be missing. Now remove half or so of the above entries, but keep the legit user, and rebuild the server's yp database. Execute 'groups' or 'id' against the user on the FreeBSD 6 client and the test group should show up. >Fix: The included patch should do the trick. The problem is that nis_group() does not check for an ERANGE error code when coming back from __gr_parse_entry(). Rather, it just summarily continues on through the loop and tries to grab the next entry. This patch works by saving the old NIS key in the NIS state structure rather than replacing it immediately with the new key. The new key is saved off so long as an ERANGE is not encountered by __gr_parse_entr(). If this happens, the "erange" trapdoor is taken out of nis_group(). I tried to be careful with memory allocation/dealloc, but a careful scan from a second (or more) set of eyes is always a good idea. The patch below should apply cleanly to the HEAD for file src/lib/libc/getgrent.c --- getgrent.c.diff begins here --- --- getgrent.c Fri May 5 15:46:28 2006 +++ /usr/src/lib/libc/gen/getgrent.c Fri May 5 15:44:13 2006 @@ -967,7 +967,6 @@ int *errnop, keylen, resultlen, rv; name = NULL; - key = NULL; gid = (gid_t)-1; how = (enum nss_lookup_type)mdata; switch (how) { @@ -1017,16 +1016,19 @@ result = NULL; if (how == nss_lt_all) { if (st->key == NULL) - rv = yp_first(st->domain, map, &key, - &keylen, &result, &resultlen); + rv = yp_first(st->domain, map, &st->key, + &st->keylen, &result, &resultlen); else { - rv = yp_next(st->domain, map, - st->key, st->keylen, - &key, &keylen, &result, &resultlen); + key = st->key; + keylen = st->keylen; + st->key = NULL; + rv = yp_next(st->domain, map, key, keylen, + &st->key, &st->keylen, &result, + &resultlen); + free(key); } if (rv != 0) { free(result); - free(key); free(st->key); st->key = NULL; if (rv == YPERR_NOMORE) { @@ -1052,35 +1054,16 @@ * terminator, alignment padding, and one (char *) * pointer for the member list terminator. */ - if (resultlen >= bufsize - _ALIGNBYTES - sizeof(char *)) { - if (how == nss_lt_all) { - free(key); - } - goto erange; - } + if (resultlen >= bufsize - _ALIGNBYTES - sizeof(char *)) + goto erange; memcpy(buffer, result, resultlen); buffer[resultlen] = '\0'; free(result); rv = __gr_match_entry(buffer, resultlen, how, name, gid); - if (rv == NS_SUCCESS) { + if (rv == NS_SUCCESS) rv = __gr_parse_entry(buffer, resultlen, grp, &buffer[resultlen+1], bufsize - resultlen - 1, errnop); - if (*errnop == ERANGE) { - if (how == nss_lt_all) { - free(key); - } - goto erange; - } - } - if (how == nss_lt_all) { - if (st->key != NULL) { - free(st->key); - } - st->key = key; - st->keylen = keylen; - key = NULL; - } } while (how == nss_lt_all && !(rv & NS_TERMINATE)); fin: if (rv == NS_SUCCESS && retval != NULL) --- getgrent.c.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted: