bin/170413: mountd: correct handling of -alldirs option and segmentation fault for -sec option

Andrey Simonenko simon at comsys.ntu-kpi.kiev.ua
Mon Aug 6 10:40:05 UTC 2012


>Number:         170413
>Category:       bin
>Synopsis:       mountd: correct handling of -alldirs option and segmentation fault for -sec option
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Aug 06 10:40:05 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Andrey Simonenko
>Release:        FreeBSD 10.0-CURRENT amd64
>Organization:
FreeBSD-gnats-submit at freebsd.org
>Environment:
>Description:

According to the exports(5) manual page if a line starts with a single
pathname of the root of the file system followed by the -alldirs option.
This option allows to specify that this is a file system export, does not
matter whether it is mounted right now or will be mounted in future.

mountd starting from the 1.84 revision of the usr.sbin/mountd/mountd.c
file ignores the -alldirs option (> 5 years ago).  It silently treats
the given pathname as a directory name and exports the entire file
system this directory belongs to.  Actually it has to export the given
pathname only if it is a mount point.  This is a security issue, since
mountd violates exports(5) rules.

Also the following update corrects segmentation fault if the -sec option
is given without an argument.

>How-To-Repeat:

Create the /etc/exports file with this content:

/cdrom -alldirs

Suppose /cdrom is not a mount point, now run mountd and try to mount
the <server>:/ NFS export, you will get access to the root file system.

>Fix:
--- mountd.c.orig	2012-01-20 13:19:39.000000000 +0200
+++ mountd.c	2012-08-06 12:50:52.000000000 +0300
@@ -174,8 +174,8 @@ static void	complete_service(struct netc
 static void	clearout_service(void);
 void	del_mlist(char *hostp, char *dirp);
 struct dirlist *dirp_search(struct dirlist *, char *);
-int	do_mount(struct exportlist *, struct grouplist *, int,
-		struct xucred *, char *, int, struct statfs *);
+static int do_mount(const struct exportlist *, const struct grouplist *, int,
+		const struct xucred *, const char *, struct statfs *);
 int	do_opt(char **, char **, struct exportlist *, struct grouplist *,
 				int *, int *, struct xucred *);
 struct	exportlist *ex_search(fsid_t *);
@@ -1333,7 +1333,7 @@ get_exportlist_one(void)
 	struct statfs fsb;
 	struct xucred anon;
 	char *cp, *endcp, *dirp, *hst, *usr, *dom, savedc;
-	int len, has_host, exflags, got_nondir, dirplen, netgrp;
+	int len, has_host, exflags, got_nondir, netgrp;
 
 	v4root_phase = 0;
 	dirhead = (struct dirlist *)NULL;
@@ -1473,7 +1473,6 @@ get_exportlist_one(void)
 				     * Add dirpath to export mount point.
 				     */
 				    dirp = add_expdir(&dirhead, cp, len);
-				    dirplen = len;
 				}
 			    } else {
 				getexp_err(ep, tgrp);
@@ -1567,8 +1566,7 @@ get_exportlist_one(void)
 		 */
 		grp = tgrp;
 		do {
-			if (do_mount(ep, grp, exflags, &anon, dirp, dirplen,
-			    &fsb)) {
+			if (do_mount(ep, grp, exflags, &anon, dirp, &fsb)) {
 				getexp_err(ep, tgrp);
 				goto nextline;
 			}
@@ -2183,7 +2181,7 @@ do_opt(char **cpp, char **endcpp, struct
 			ep->ex_indexfile = strdup(cpoptarg);
 		} else if (!strcmp(cpopt, "quiet")) {
 			opt_flags |= OP_QUIET;
-		} else if (!strcmp(cpopt, "sec")) {
+		} else if (cpoptarg != NULL && !strcmp(cpopt, "sec")) {
 			if (parsesec(cpoptarg, ep))
 				return (1);
 			opt_flags |= OP_SEC;
@@ -2330,62 +2328,64 @@ out_of_mem(void)
  * Do the nmount() syscall with the update flag to push the export info into
  * the kernel.
  */
-int
-do_mount(struct exportlist *ep, struct grouplist *grp, int exflags,
-    struct xucred *anoncrp, char *dirp, int dirplen, struct statfs *fsb)
+static int
+do_mount(const struct exportlist *ep, const struct grouplist *grp, int exflags,
+    const struct xucred *anoncrp, const char *dirp, struct statfs *fsb)
 {
-	struct statfs fsb1;
-	struct addrinfo *ai;
-	struct export_args ea, *eap;
-	char errmsg[255];
-	char *cp;
-	int done;
-	char savedc;
-	struct iovec *iov;
-	int i, iovlen;
-	int ret;
+	char errmsg[256];
 	struct nfsex_args nfsea;
+	const struct addrinfo *ai;
+	struct export_args *eap;
+	struct iovec *iov;
+	int i, ret, iovlen;
 
-	if (run_v4server > 0)
-		eap = &nfsea.export;
-	else
-		eap = &ea;
-
-	cp = NULL;
-	savedc = '\0';
 	iov = NULL;
-	iovlen = 0;
-	ret = 0;
+	eap = &nfsea.export;
+	if (v4root_phase == 0) {
+		if ((opt_flags & OP_ALLDIRS) &&
+		    strcmp(dirp, fsb->f_mntonname) != 0) {
+			if (!(opt_flags & OP_QUIET))
+				syslog(LOG_ERR, "-alldirs requested but %s "
+				    "is not a file system mount point", dirp);
+			return (1);
+		}
+		bzero(errmsg, sizeof(errmsg));
+		iovlen = 0;
+		build_iovec(&iov, &iovlen, "fstype", fsb->f_fstypename,
+		    (size_t)-1);
+		build_iovec(&iov, &iovlen, "fspath", fsb->f_mntonname,
+		    (size_t)-1);
+		build_iovec(&iov, &iovlen, "from", fsb->f_mntfromname,
+		    (size_t)-1);
+		build_iovec(&iov, &iovlen, "update", NULL, 0);
+		build_iovec(&iov, &iovlen, "export", eap, sizeof(*eap));
+		build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
+		if (iovlen < 0) {
+			syslog(LOG_ERR, "build_iovec: %m");
+			return (1);
+		}
+	} else {
+		if (run_v4server == 0)
+			return (0);
+		nfsea.fspec = v4root_dirpath;
+	}
 
-	bzero(eap, sizeof (struct export_args));
-	bzero(errmsg, sizeof(errmsg));
+	bzero(eap, sizeof(*eap));
 	eap->ex_flags = exflags;
 	eap->ex_anon = *anoncrp;
 	eap->ex_indexfile = ep->ex_indexfile;
-	if (grp->gr_type == GT_HOST)
-		ai = grp->gr_ptr.gt_addrinfo;
-	else
-		ai = NULL;
-	eap->ex_numsecflavors = ep->ex_numsecflavors;
-	for (i = 0; i < eap->ex_numsecflavors; i++)
-		eap->ex_secflavors[i] = ep->ex_secflavors[i];
-	if (eap->ex_numsecflavors == 0) {
+	ai = grp->gr_type == GT_HOST ? grp->gr_ptr.gt_addrinfo : NULL;
+	if (ep->ex_numsecflavors == 0) {
 		eap->ex_numsecflavors = 1;
 		eap->ex_secflavors[0] = AUTH_SYS;
-	}
-	done = FALSE;
-
-	if (v4root_phase == 0) {
-		build_iovec(&iov, &iovlen, "fstype", NULL, 0);
-		build_iovec(&iov, &iovlen, "fspath", NULL, 0);
-		build_iovec(&iov, &iovlen, "from", NULL, 0);
-		build_iovec(&iov, &iovlen, "update", NULL, 0);
-		build_iovec(&iov, &iovlen, "export", eap,
-		    sizeof (struct export_args));
-		build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
+	} else {
+		eap->ex_numsecflavors = ep->ex_numsecflavors;
+		for (i = 0; i < eap->ex_numsecflavors; i++)
+			eap->ex_secflavors[i] = ep->ex_secflavors[i];
 	}
 
-	while (!done) {
+	ret = 1;
+	for (;;) {
 		switch (grp->gr_type) {
 		case GT_HOST:
 			if (ai->ai_addr->sa_family == AF_INET6 && have_v6 == 0)
@@ -2398,13 +2398,14 @@ do_mount(struct exportlist *ep, struct g
 			if (grp->gr_ptr.gt_net.nt_net.ss_family == AF_INET6 &&
 			    have_v6 == 0)
 				goto skip;
-			eap->ex_addr =
-			    (struct sockaddr *)&grp->gr_ptr.gt_net.nt_net;
-			eap->ex_addrlen =
-			    ((struct sockaddr *)&grp->gr_ptr.gt_net.nt_net)->sa_len;
-			eap->ex_mask =
-			    (struct sockaddr *)&grp->gr_ptr.gt_net.nt_mask;
-			eap->ex_masklen = ((struct sockaddr *)&grp->gr_ptr.gt_net.nt_mask)->sa_len;
+			eap->ex_addr = (struct sockaddr *)
+			    &grp->gr_ptr.gt_net.nt_net;
+			eap->ex_addrlen = ((struct sockaddr *)
+			    &grp->gr_ptr.gt_net.nt_net)->sa_len;
+			eap->ex_mask = (struct sockaddr *)
+			    &grp->gr_ptr.gt_net.nt_mask;
+			eap->ex_masklen = ((struct sockaddr *)
+			    &grp->gr_ptr.gt_net.nt_mask)->sa_len;
 			break;
 		case GT_DEFAULT:
 			eap->ex_addr = NULL;
@@ -2415,145 +2416,61 @@ do_mount(struct exportlist *ep, struct g
 		case GT_IGNORE:
 			ret = 0;
 			goto error_exit;
-			break;
 		default:
 			syslog(LOG_ERR, "bad grouptype");
-			if (cp)
-				*cp = savedc;
-			ret = 1;
 			goto error_exit;
-		};
+		}
 
 		/*
-		 * For V4:, use the nfssvc() syscall, instead of mount().
+		 * For V4:, use the nfssvc() syscall, instead of nmount().
 		 */
-		if (v4root_phase == 2) {
-			nfsea.fspec = v4root_dirpath;
-			if (run_v4server > 0 &&
-			    nfssvc(NFSSVC_V4ROOTEXPORT, (caddr_t)&nfsea) < 0) {
-				syslog(LOG_ERR, "Exporting V4: failed");
-				return (2);
+		if (v4root_phase != 0) {
+			if (nfssvc(NFSSVC_V4ROOTEXPORT, &nfsea) < 0) {
+				syslog(LOG_ERR, "Exporting V4: failed: %m");
+				goto error_exit;
 			}
 		} else {
-			/*
-			 * XXX:
-			 * Maybe I should just use the fsb->f_mntonname path
-			 * instead of looping back up the dirp to the mount
-			 * point??
-			 * Also, needs to know how to export all types of local
-			 * exportable filesystems and not just "ufs".
-			 */
-			iov[1].iov_base = fsb->f_fstypename; /* "fstype" */
-			iov[1].iov_len = strlen(fsb->f_fstypename) + 1;
-			iov[3].iov_base = fsb->f_mntonname; /* "fspath" */
-			iov[3].iov_len = strlen(fsb->f_mntonname) + 1;
-			iov[5].iov_base = fsb->f_mntfromname; /* "from" */
-			iov[5].iov_len = strlen(fsb->f_mntfromname) + 1;
-	
-			while (nmount(iov, iovlen, fsb->f_flags) < 0) {
-				if (cp)
-					*cp-- = savedc;
-				else
-					cp = dirp + dirplen - 1;
-				if (opt_flags & OP_QUIET) {
-					ret = 1;
-					goto error_exit;
-				}
-				if (errno == EPERM) {
-					if (debug)
-						warnx("can't change attributes for %s",
-						    dirp);
-					syslog(LOG_ERR,
-					   "can't change attributes for %s",
-					    dirp);
-					ret = 1;
-					goto error_exit;
-				}
-				if (opt_flags & OP_ALLDIRS) {
-					if (errno == EINVAL)
-						syslog(LOG_ERR,
-		"-alldirs requested but %s is not a filesystem mountpoint",
-						    dirp);
-					else
-						syslog(LOG_ERR,
-						    "could not remount %s: %m",
-						    dirp);
-					ret = 1;
-					goto error_exit;
-				}
-				/* back up over the last component */
-				while (*cp == '/' && cp > dirp)
-					cp--;
-				while (*(cp - 1) != '/' && cp > dirp)
-					cp--;
-				if (cp == dirp) {
-					if (debug)
-						warnx("mnt unsucc");
-					syslog(LOG_ERR, "can't export %s %s",
-					    dirp, errmsg);
-					ret = 1;
-					goto error_exit;
-				}
-				savedc = *cp;
-				*cp = '\0';
-				/*
-				 * Check that we're still on the same
-				 * filesystem.
-				 */
-				if (statfs(dirp, &fsb1) != 0 ||
-				    bcmp(&fsb1.f_fsid, &fsb->f_fsid,
-				    sizeof (fsb1.f_fsid)) != 0) {
-					*cp = savedc;
-					syslog(LOG_ERR,
-					    "can't export %s %s", dirp,
-					    errmsg);
-					ret = 1;
-					goto error_exit;
-				}
+			if (nmount(iov, iovlen, fsb->f_flags) < 0) {
+				ret = 1;
+				if (!(opt_flags & OP_QUIET))
+					syslog(LOG_ERR, "can't change "
+					    "attributes for %s: %m", dirp);
+				goto error_exit;
 			}
 		}
-
-		/*
-		 * For the experimental server:
-		 * If this is the public directory, get the file handle
-		 * and load it into the kernel via the nfssvc() syscall.
-		 */
-		if (run_v4server > 0 && (exflags & MNT_EXPUBLIC) != 0) {
-			fhandle_t fh;
-			char *public_name;
-
-			if (eap->ex_indexfile != NULL)
-				public_name = eap->ex_indexfile;
-			else
-				public_name = dirp;
-			if (getfh(public_name, &fh) < 0)
-				syslog(LOG_ERR,
-				    "Can't get public fh for %s", public_name);
-			else if (nfssvc(NFSSVC_PUBLICFH, (caddr_t)&fh) < 0)
-				syslog(LOG_ERR,
-				    "Can't set public fh for %s", public_name);
-			else
-				has_publicfh = 1;
-		}
 skip:
 		if (ai != NULL)
 			ai = ai->ai_next;
 		if (ai == NULL)
-			done = TRUE;
+			break;
+	}
+
+	/*
+	 * For the experimental server:
+	 * If this is the public directory, get the file handle
+	 * and load it into the kernel via the nfssvc() syscall.
+	 */
+	if (run_v4server > 0 && (exflags & MNT_EXPUBLIC)) {
+		fhandle_t fh;
+		const char *public_name;
+
+		public_name = eap->ex_indexfile != NULL ?
+		    eap->ex_indexfile : dirp;
+		if (getfh(public_name, &fh) < 0)
+			syslog(LOG_ERR, "Can't get public fh for %s: %m",
+			    public_name);
+		else if (nfssvc(NFSSVC_PUBLICFH, &fh) < 0)
+			syslog(LOG_ERR, "Can't set public fh for %s: %m",
+			    public_name);
+		else
+			has_publicfh = 1;
 	}
-	if (cp)
-		*cp = savedc;
+	ret = 0;
 error_exit:
-	/* free strings allocated by strdup() in getmntopts.c */
+	/* Free data allocated by build_iovec(). */
 	if (iov != NULL) {
-		free(iov[0].iov_base); /* fstype */
-		free(iov[2].iov_base); /* fspath */
-		free(iov[4].iov_base); /* from */
-		free(iov[6].iov_base); /* update */
-		free(iov[8].iov_base); /* export */
-		free(iov[10].iov_base); /* errmsg */
-
-		/* free iov, allocated by realloc() */
+		for (i = 0; i <= 10; i += 2)
+			free(iov[i].iov_base);
 		free(iov);
 	}
 	return (ret);
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list