On Thu, Jun 14, 2012 at 09:48:15PM +0000, Guy Helmer wrote: > Author: ghelmer > Date: Thu Jun 14 21:48:14 2012 > New Revision: 237106 > URL: http://svn.freebsd.org/changeset/base/237106 > > Log: > MFC 235739-235740,236402: > Apply style(9) to return and switch/case statements. > > Add checks for memory allocation failures in appropriate places, and > avoid creating bad entries in the grp list as a result of memory allocation > failures while building new entries. > > Style(9) improvements: remove unnecessary parenthesis, improve order > of local variable declarations, remove bogus casts, and resolve long > lines. > > PR: bin/83340 > > Modified: > stable/9/lib/libc/gen/getnetgrent.c > Directory Properties: > stable/9/lib/libc/ (props changed) > This broke mountd(8) for me. It dies with SIGSEGV on start. Autopsy has shown that this happen due to free(3) on an unallocated pointer.
I have two netgroups in my /etc/netgroup: testboxes (solo.home, , ) (x.home, , ) (nv.home, , ) (sandy.home, , ) (tom.home, ,) laptops (alf.home, , ) (alf.home-air, , ) (sirion.home, , ) (sirion.home-air, , ) The http://people.freebsd.org/~kib/misc/netgr.c shows the issue. Run it as "netgr testboxes laptops". Your change below makes lp->l_lines pointer for already processed groups invalid because you reallocf(3) the memory pointed by it. Then 1. accessing the l_lines later causes undefined behaviour; 2. free(3) call in endnetgrent() dies because pointer is dandling. > @@ -595,15 +615,15 @@ read_for_group(const char *group) > } else > cont = 0; > if (len > 0) { > - linep = (char *)malloc(olen + len + 1); > - if (olen > 0) { > - bcopy(olinep, linep, olen); > - free(olinep); > + linep = reallocf(linep, olen + len + 1); > + if (linep == NULL) { > + free(lp->l_groupname); > + free(lp); > + return (NULL); > } > bcopy(pos, linep + olen, len); > olen += len; > *(linep + olen) = '\0'; > - olinep = linep; > } > if (cont) { > if (fgets(line, LINSIZ, netf)) { Below is partial revert of your changes that cures mountd for me. Also, the second patch does some further cleanup of the getnetgrent.c. Do you agree ? commit d5cb6720e8d12b281e89f2487561fe95addd0e34 Author: Konstantin Belousov <k...@freebsd.org> Date: Sat Jun 16 00:21:11 2012 +0300 Partially revert r235740, namely, allocate separate blocks of memory for each group' l_line. Using the reallocf(3) invalidates l_line's of the previously read groups. diff --git a/lib/libc/gen/getnetgrent.c b/lib/libc/gen/getnetgrent.c index 933a7d3..8cad6c4 100644 --- a/lib/libc/gen/getnetgrent.c +++ b/lib/libc/gen/getnetgrent.c @@ -538,7 +538,7 @@ parse_netgrp(const char *group) static struct linelist * read_for_group(const char *group) { - char *pos, *spos, *linep; + char *pos, *spos, *linep, *olinep; int len, olen; int cont; struct linelist *lp; @@ -615,15 +615,20 @@ read_for_group(const char *group) } else cont = 0; if (len > 0) { - linep = reallocf(linep, olen + len + 1); + linep = malloc(olen + len + 1); if (linep == NULL) { free(lp->l_groupname); free(lp); return (NULL); } + if (olen > 0) { + bcopy(olinep, linep, olen); + free(olinep); + } bcopy(pos, linep + olen, len); olen += len; *(linep + olen) = '\0'; + olinep = linep; } if (cont) { if (fgets(line, LINSIZ, netf)) { commit 5946753b4b094d5f7ac41792df64915434567fd8 Author: Konstantin Belousov <k...@freebsd.org> Date: Sat Jun 16 00:23:01 2012 +0300 More style. diff --git a/lib/libc/gen/getnetgrent.c b/lib/libc/gen/getnetgrent.c index 8cad6c4..d94fff7 100644 --- a/lib/libc/gen/getnetgrent.c +++ b/lib/libc/gen/getnetgrent.c @@ -161,8 +161,7 @@ setnetgrent(const char *group) if (group == NULL || !strlen(group)) return; - if (grouphead.gr == (struct netgrp *)0 || - strcmp(group, grouphead.grname)) { + if (grouphead.gr == NULL || strcmp(group, grouphead.grname)) { endnetgrent(); #ifdef YP /* Presumed guilty until proven innocent. */ @@ -172,7 +171,7 @@ setnetgrent(const char *group) * use NIS exclusively. */ if (((stat(_PATH_NETGROUP, &_yp_statp) < 0) && - errno == ENOENT) || _yp_statp.st_size == 0) + errno == ENOENT) || _yp_statp.st_size == 0) _use_only_yp = _netgr_yp_enabled = 1; if ((netf = fopen(_PATH_NETGROUP,"r")) != NULL ||_use_only_yp){ /* @@ -247,27 +246,24 @@ endnetgrent(void) lp = lp->l_next; free(olp->l_groupname); free(olp->l_line); - free((char *)olp); + free(olp); } - linehead = (struct linelist *)0; + linehead = NULL; if (grouphead.grname) { free(grouphead.grname); - grouphead.grname = (char *)0; + grouphead.grname = NULL; } gp = grouphead.gr; while (gp) { ogp = gp; gp = gp->ng_next; - if (ogp->ng_str[NG_HOST]) - free(ogp->ng_str[NG_HOST]); - if (ogp->ng_str[NG_USER]) - free(ogp->ng_str[NG_USER]); - if (ogp->ng_str[NG_DOM]) - free(ogp->ng_str[NG_DOM]); - free((char *)ogp); + free(ogp->ng_str[NG_HOST]); + free(ogp->ng_str[NG_USER]); + free(ogp->ng_str[NG_DOM]); + free(ogp); } - grouphead.gr = (struct netgrp *)0; - nextgrp = (struct netgrp *)0; + grouphead.gr = NULL; + nextgrp = NULL; #ifdef YP _netgr_yp_enabled = 0; #endif @@ -282,7 +278,7 @@ _listmatch(const char *list, const char *group, int len) int glen = strlen(group); /* skip possible leading whitespace */ - while(isspace((unsigned char)*ptr)) + while (isspace((unsigned char)*ptr)) ptr++; while (ptr < list + len) { @@ -291,7 +287,7 @@ _listmatch(const char *list, const char *group, int len) ptr++; if (strncmp(cptr, group, glen) == 0 && glen == (ptr - cptr)) return (1); - while(*ptr == ',' || isspace((unsigned char)*ptr)) + while (*ptr == ',' || isspace((unsigned char)*ptr)) ptr++; } @@ -436,8 +432,7 @@ parse_netgrp(const char *group) break; lp = lp->l_next; } - if (lp == (struct linelist *)0 && - (lp = read_for_group(group)) == (struct linelist *)0) + if (lp == NULL && (lp = read_for_group(group)) == NULL) return (1); if (lp->l_parsed) { #ifdef DEBUG
pgpEQvo8EQIaP.pgp
Description: PGP signature
