Date: Sat, 16 Jun 2012 00:26:51 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Guy Helmer <ghelmer@freebsd.org> Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-9@freebsd.org Subject: Re: svn commit: r237106 - stable/9/lib/libc/gen Message-ID: <20120615212651.GH2337@deviant.kiev.zoral.com.ua> In-Reply-To: <201206142148.q5ELmFul023002@svn.freebsd.org> References: <201206142148.q5ELmFul023002@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--WFYXJKyHTUiGqOmp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 >=20 > Log: > MFC 235739-235740,236402: > Apply style(9) to return and switch/case statements. > =20 > Add checks for memory allocation failures in appropriate places, and > avoid creating bad entries in the grp list as a result of memory alloca= tion > failures while building new entries. > =20 > Style(9) improvements: remove unnecessary parenthesis, improve order > of local variable declarations, remove bogus casts, and resolve long > lines. > =20 > PR: bin/83340 >=20 > Modified: > stable/9/lib/libc/gen/getnetgrent.c > Directory Properties: > stable/9/lib/libc/ (props changed) >=20 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-ai= r, , ) 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 =3D 0; > if (len > 0) { > - linep =3D (char *)malloc(olen + len + 1); > - if (olen > 0) { > - bcopy(olinep, linep, olen); > - free(olinep); > + linep =3D reallocf(linep, olen + len + 1); > + if (linep =3D=3D NULL) { > + free(lp->l_groupname); > + free(lp); > + return (NULL); > } > bcopy(pos, linep + olen, len); > olen +=3D len; > *(linep + olen) =3D '\0'; > - olinep =3D 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 <kib@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 =3D 0; if (len > 0) { - linep =3D reallocf(linep, olen + len + 1); + linep =3D malloc(olen + len + 1); if (linep =3D=3D NULL) { free(lp->l_groupname); free(lp); return (NULL); } + if (olen > 0) { + bcopy(olinep, linep, olen); + free(olinep); + } bcopy(pos, linep + olen, len); olen +=3D len; *(linep + olen) =3D '\0'; + olinep =3D linep; } if (cont) { if (fgets(line, LINSIZ, netf)) { commit 5946753b4b094d5f7ac41792df64915434567fd8 Author: Konstantin Belousov <kib@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 =3D=3D NULL || !strlen(group)) return; =20 - if (grouphead.gr =3D=3D (struct netgrp *)0 || - strcmp(group, grouphead.grname)) { + if (grouphead.gr =3D=3D 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 =3D=3D ENOENT) || _yp_statp.st_size =3D=3D 0) + errno =3D=3D ENOENT) || _yp_statp.st_size =3D=3D 0) _use_only_yp =3D _netgr_yp_enabled =3D 1; if ((netf =3D fopen(_PATH_NETGROUP,"r")) !=3D NULL ||_use_only_yp){ /* @@ -247,27 +246,24 @@ endnetgrent(void) lp =3D lp->l_next; free(olp->l_groupname); free(olp->l_line); - free((char *)olp); + free(olp); } - linehead =3D (struct linelist *)0; + linehead =3D NULL; if (grouphead.grname) { free(grouphead.grname); - grouphead.grname =3D (char *)0; + grouphead.grname =3D NULL; } gp =3D grouphead.gr; while (gp) { ogp =3D gp; gp =3D 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 =3D (struct netgrp *)0; - nextgrp =3D (struct netgrp *)0; + grouphead.gr =3D NULL; + nextgrp =3D NULL; #ifdef YP _netgr_yp_enabled =3D 0; #endif @@ -282,7 +278,7 @@ _listmatch(const char *list, const char *group, int len) int glen =3D strlen(group); =20 /* skip possible leading whitespace */ - while(isspace((unsigned char)*ptr)) + while (isspace((unsigned char)*ptr)) ptr++; =20 while (ptr < list + len) { @@ -291,7 +287,7 @@ _listmatch(const char *list, const char *group, int len) ptr++; if (strncmp(cptr, group, glen) =3D=3D 0 && glen =3D=3D (ptr - cptr)) return (1); - while(*ptr =3D=3D ',' || isspace((unsigned char)*ptr)) + while (*ptr =3D=3D ',' || isspace((unsigned char)*ptr)) ptr++; } =20 @@ -436,8 +432,7 @@ parse_netgrp(const char *group) break; lp =3D lp->l_next; } - if (lp =3D=3D (struct linelist *)0 && - (lp =3D read_for_group(group)) =3D=3D (struct linelist *)0) + if (lp =3D=3D NULL && (lp =3D read_for_group(group)) =3D=3D NULL) return (1); if (lp->l_parsed) { #ifdef DEBUG --WFYXJKyHTUiGqOmp Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk/bqJsACgkQC3+MBN1Mb4g0OgCeICn8JafwTeuEFyX6W/832Vdo 5t8An0ruEzvPgjB1ESel21VkvL/cSyH0 =AIOB -----END PGP SIGNATURE----- --WFYXJKyHTUiGqOmp--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120615212651.GH2337>