svn commit: r237106 - stable/9/lib/libc/gen

Konstantin Belousov kostikbel at gmail.com
Fri Jun 15 21:27:05 UTC 2012


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 <kib at 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 <kib at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-stable-9/attachments/20120615/9ff11791/attachment.pgp


More information about the svn-src-stable-9 mailing list