bin/170447: mountd: correct handling of -n, -h and -p command line options

Andrey Simonenko simon at comsys.ntu-kpi.kiev.ua
Tue Aug 7 12:40:07 UTC 2012


>Number:         170447
>Category:       bin
>Synopsis:       mountd: correct handling of -n, -h and -p command line options
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Aug 07 12:40:06 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Andrey Simonenko
>Release:        FreeBSD 10.0-CURRENT amd64
>Organization:
>Environment:
>Description:

Description of the following update:

1. For the -p option: use strtonum() instead of strtoul(), check the return
   value for the malloc() call.

2. Corrected memory leaks when the netconfig database is accessed, use two
   netconfig handles to see the same netconfig database data, check return
   values from setnetconfig() and endnetconfig().

3. For the -h option: simplify code.

4. For the -n option: use sysctl name depending on used NFS server
   implementation.

5. Use correct data type for the got_sighup variable and remove unnecessary
   assignment in get_line().

>How-To-Repeat:
>Fix:
--- mountd.8.orig	2011-04-26 15:59:25.000000000 +0300
+++ mountd.8	2012-08-07 12:44:08.000000000 +0300
@@ -96,7 +96,8 @@
 Allow non-root mount requests to be served.
 This should only be specified if there are clients such as PC's,
 that require it.
-It will automatically clear the vfs.nfsrv.nfs_privport sysctl flag, which
+It will automatically clear the vfs.nfsd.nfs_privport or
+vfs.nfsrv.nfs_privport sysctl flag, which
 controls if the kernel will accept NFS requests from reserved ports only.
 .It Fl o
 This flag forces the system to run the old NFS server, which does not

---------------------------------------------------------------------------

--- mountd.c.orig	2012-08-07 12:44:08.000000000 +0300
+++ mountd.c	2012-08-07 12:42:48.000000000 +0300
@@ -169,8 +169,9 @@ int	check_dirpath(char *);
 int	check_options(struct dirlist *);
 int	checkmask(struct sockaddr *sa);
 int	chk_host(struct dirlist *, struct sockaddr *, int *, int *);
-static int	create_service(struct netconfig *nconf);
-static void	complete_service(struct netconfig *nconf, char *port_str);
+static int	create_service(const struct netconfig *nconf);
+static void	complete_service(const struct netconfig *nconf,
+		    const char *port_str);
 static void	clearout_service(void);
 void	del_mlist(char *hostp, char *dirp);
 struct dirlist *dirp_search(struct dirlist *, char *);
@@ -220,7 +221,7 @@ struct mountlist *mlhead;
 struct grouplist *grphead;
 char *exnames_default[2] = { _PATH_EXPORTS, NULL };
 char **exnames;
-char **hosts = NULL;
+static const char **hosts = NULL;
 struct xucred def_anon = {
 	XUCRED_VERSION,
 	(uid_t)-2,
@@ -233,7 +234,7 @@ int resvport_only = 1;
 int nhosts = 0;
 int dir_only = 1;
 int dolog = 0;
-int got_sighup = 0;
+static volatile sig_atomic_t got_sighup = 0;
 int xcreated = 0;
 
 char *svcport_str = NULL;
@@ -282,12 +283,11 @@ int
 main(int argc, char **argv)
 {
 	fd_set readfds;
-	struct netconfig *nconf;
-	char *endptr, **hosts_bak;
-	void *nc_handle;
+	const struct netconfig *nconf;
+	const char *errstr;
+	void *nc_handle1, *nc_handle2;
 	pid_t otherpid;
-	in_port_t svcport;
-	int c, k, s;
+	int c, s;
 	int maxrec = RPC_MAXDATASIZE;
 	int attempt_cnt, port_len, port_pos, ret;
 	char **port_list;
@@ -330,33 +330,18 @@ main(int argc, char **argv)
 			run_v4server = 0;
 			break;
 		case 'p':
-			endptr = NULL;
-			svcport = (in_port_t)strtoul(optarg, &endptr, 10);
-			if (endptr == NULL || *endptr != '\0' ||
-			    svcport == 0 || svcport >= IPPORT_MAX)
+			(void)strtonum(optarg, 1, IPPORT_MAX, &errstr);
+			if (errstr != NULL)
 				usage();
 			svcport_str = strdup(optarg);
+			if (svcport_str == NULL)
+				out_of_mem();
 			break;
 		case 'h':
-			++nhosts;
-			hosts_bak = hosts;
-			hosts_bak = realloc(hosts, nhosts * sizeof(char *));
-			if (hosts_bak == NULL) {
-				if (hosts != NULL) {
-					for (k = 0; k < nhosts; k++) 
-						free(hosts[k]);
-					free(hosts);
-					out_of_mem();
-				}
-			}
-			hosts = hosts_bak;
-			hosts[nhosts - 1] = strdup(optarg);
-			if (hosts[nhosts - 1] == NULL) {
-				for (k = 0; k < (nhosts - 1); k++) 
-					free(hosts[k]);
-				free(hosts);
+			hosts = realloc(hosts, ++nhosts * sizeof(*hosts));
+			if (hosts == NULL)
 				out_of_mem();
-			}
+			hosts[nhosts - 1] = optarg;
 			break;
 		default:
 			usage();
@@ -412,11 +397,12 @@ main(int argc, char **argv)
 	rpc_control(RPC_SVC_CONNMAXREC_SET, &maxrec);
 
 	if (!resvport_only) {
-		if (sysctlbyname("vfs.nfsrv.nfs_privport", NULL, NULL,
-		    &resvport_only, sizeof(resvport_only)) != 0 &&
-		    errno != ENOENT) {
+		if (sysctlbyname(run_v4server ?
+		    "vfs.nfsd.nfs_privport" : "vfs.nfsrv.nfs_privport",
+		    NULL, (size_t *)NULL, &resvport_only,
+		    sizeof(resvport_only)) != 0) {
 			syslog(LOG_ERR, "sysctl: %m");
-			exit(1);
+			return (EXIT_FAILURE);
 		}
 	}
 
@@ -426,38 +412,18 @@ main(int argc, char **argv)
 	 * list.
 	 */
 	if (nhosts == 0) {
-		hosts = malloc(sizeof(char**));
+		hosts = malloc(sizeof(*hosts));
 		if (hosts == NULL)
 			out_of_mem();
 		hosts[0] = "*";
 		nhosts = 1;
 	} else {
-		hosts_bak = hosts;
-		if (have_v6) {
-			hosts_bak = realloc(hosts, (nhosts + 2) *
-			    sizeof(char *));
-			if (hosts_bak == NULL) {
-				for (k = 0; k < nhosts; k++)
-					free(hosts[k]);
-		    		free(hosts);
-		    		out_of_mem();
-			} else
-				hosts = hosts_bak;
-			nhosts += 2;
+		nhosts += have_v6 ? 2 : 1;
+		hosts = realloc(hosts, nhosts * sizeof(*hosts));
+		if (hosts == NULL)
+			out_of_mem();
+		if (have_v6)
 			hosts[nhosts - 2] = "::1";
-		} else {
-			hosts_bak = realloc(hosts, (nhosts + 1) * sizeof(char *));
-			if (hosts_bak == NULL) {
-				for (k = 0; k < nhosts; k++)
-					free(hosts[k]);
-				free(hosts);
-				out_of_mem();
-			} else {
-				nhosts += 1;
-				hosts = hosts_bak;
-			}
-		}
-
 		hosts[nhosts - 1] = "127.0.0.1";
 	}
 
@@ -466,9 +432,15 @@ main(int argc, char **argv)
 	sock_fd = NULL;
 	port_list = NULL;
 	port_len = 0;
-	nc_handle = setnetconfig();
-	while ((nconf = getnetconfig(nc_handle))) {
-		if (nconf->nc_flag & NC_VISIBLE) {
+	for (;;) {
+		nc_handle1 = setnetconfig();
+		if (nc_handle1 == NULL) {
+			syslog(LOG_ERR, "main: setnetconfig: %s", nc_sperror());
+			return (EXIT_FAILURE);
+		}
+		while ((nconf = getnetconfig(nc_handle1)) != NULL) {
+			if (!(nconf->nc_flag & NC_VISIBLE))
+				continue;
 			if (have_v6 == 0 && strcmp(nconf->nc_protofmly,
 			    "inet6") == 0) {
 				/* DO NOTHING */
@@ -501,8 +473,8 @@ main(int argc, char **argv)
 					free(sock_fd);
 					sock_fdcnt = 0;
 					sock_fd = NULL;
-					nc_handle = setnetconfig();
 					attempt_cnt++;
+					break;
 				} else if (mallocd_svcport != 0 &&
 				    attempt_cnt == GETPORT_MAXTRY) {
 					/*
@@ -521,6 +493,12 @@ main(int argc, char **argv)
 				}
 			}
 		}
+		if (nconf == NULL)
+			break;
+		if (endnetconfig(nc_handle1) < 0) {
+			syslog(LOG_ERR, "main: endnetconfig: %s", nc_sperror());
+			return (EXIT_FAILURE);
+		}
 	}
 
 	/*
@@ -529,8 +507,12 @@ main(int argc, char **argv)
 	 */
 	sock_fdpos = 0;
 	port_pos = 0;
-	nc_handle = setnetconfig();
-	while ((nconf = getnetconfig(nc_handle))) {
+	nc_handle2 = setnetconfig();
+	if (nc_handle2 == NULL) {
+		syslog(LOG_ERR, "main: setnetconfig: %s", nc_sperror());
+		return (EXIT_FAILURE);
+	}
+	while ((nconf = getnetconfig(nc_handle2)) != NULL) {
 		if (nconf->nc_flag & NC_VISIBLE) {
 			if (have_v6 == 0 && strcmp(nconf->nc_protofmly,
 			    "inet6") == 0) {
@@ -545,7 +527,10 @@ main(int argc, char **argv)
 				complete_service(nconf, svcport_str);
 		}
 	}
-	endnetconfig(nc_handle);
+	if (endnetconfig(nc_handle1) < 0 || endnetconfig(nc_handle2) < 0) {
+		syslog(LOG_ERR, "main: endnetconfig: %s", nc_sperror());
+		return (EXIT_FAILURE);
+	}
 	free(sock_fd);
 	if (port_list != NULL) {
 		for (port_pos = 0; port_pos < port_len; port_pos++)
@@ -588,7 +573,7 @@ main(int argc, char **argv)
  * the total count of them is maintained in sock_fdcnt.
  */
 static int
-create_service(struct netconfig *nconf)
+create_service(const struct netconfig *nconf)
 {
 	struct addrinfo hints, *res = NULL;
 	struct sockaddr_in *sin;
@@ -805,7 +790,7 @@ create_service(struct netconfig *nconf)
  * the setup and registration.
  */
 static void
-complete_service(struct netconfig *nconf, char *port_str)
+complete_service(const struct netconfig *nconf, const char *port_str)
 {
 	struct addrinfo hints, *res = NULL;
 	struct __rpc_sockinfo si;
@@ -2726,7 +2711,6 @@ get_line(void)
 	/*
 	 * Loop around ignoring blank lines and getting all continuation lines.
 	 */
-	p = line;
 	totlen = 0;
 	do {
 		if ((p = fgetln(exp_file, &len)) == NULL)
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list