From owner-freebsd-bugs@FreeBSD.ORG Mon Aug 6 10:40:05 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB278106566B for ; Mon, 6 Aug 2012 10:40:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 8D2028FC19 for ; Mon, 6 Aug 2012 10:40:05 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q76Ae5oJ018819 for ; Mon, 6 Aug 2012 10:40:05 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q76Ae5LI018818; Mon, 6 Aug 2012 10:40:05 GMT (envelope-from gnats) Resent-Date: Mon, 6 Aug 2012 10:40:05 GMT Resent-Message-Id: <201208061040.q76Ae5LI018818@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Andrey Simonenko Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 64B191065673 for ; Mon, 6 Aug 2012 10:34:34 +0000 (UTC) (envelope-from simon@comsys.ntu-kpi.kiev.ua) Received: from comsys.kpi.ua (comsys.kpi.ua [77.47.192.42]) by mx1.freebsd.org (Postfix) with ESMTP id B05E28FC16 for ; Mon, 6 Aug 2012 10:34:32 +0000 (UTC) Received: from pm513-1.comsys.kpi.ua ([10.18.52.101] helo=pm513-1.comsys.ntu-kpi.kiev.ua) by comsys.kpi.ua with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1SyKdq-0002RG-8B for FreeBSD-gnats-submit@freebsd.org; Mon, 06 Aug 2012 13:34:30 +0300 Received: by pm513-1.comsys.ntu-kpi.kiev.ua (Postfix, from userid 1001) id 3E6611CC1E; Mon, 6 Aug 2012 13:34:29 +0300 (EEST) Message-Id: <20120806103428.GA9697@pm513-1.comsys.ntu-kpi.kiev.ua> Date: Mon, 6 Aug 2012 13:34:29 +0300 From: Andrey Simonenko To: FreeBSD-gnats-submit@FreeBSD.org Cc: Subject: bin/170413: mountd: correct handling of -alldirs option and segmentation fault for -sec option X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Aug 2012 10:40:05 -0000 >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@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 :/ 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: