Date: Tue, 11 Nov 2003 14:36:17 GMT From: Peter Edwards <peter.edwards@openet-telecom.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/59177: vfs_nmount lacks parameter checks: non-root can cause panic/memory exhaustion. Message-ID: <200311111436.hABEaH2p040680@rocklobster.openet-telecom.lan> Resent-Message-ID: <200311111440.hABEeBws071657@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 59177 >Category: kern >Synopsis: vfs_nmount lacks parameter checks: non-root can cause panic/memory exhaustion. >Confidential: no >Severity: serious >Priority: high >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Nov 11 06:40:11 PST 2003 >Closed-Date: >Last-Modified: >Originator: Peter Edwards >Release: FreeBSD 5.1-CURRENT i386 >Organization: Openet Telecom >Environment: System: FreeBSD rocklobster 5.1-CURRENT FreeBSD 5.1-CURRENT #7: Mon Oct 13 21:11:46 IST 2003 petere@archie:/pub/FreeBSD/obj/pub/FreeBSD/current/src/sys/ROCKLOBSTER i386 >Description: (Marked as "high priority", because it has security implications. Originally posted to -current on 11/04/03 with subject "Bug: nmount(2) lacks parameter checking." I finally forced myself into learning enough about sendmail to get it to relay to our MX, so my send-pr should now work.) nmount() calls vfs_nmount() pretty much directly after copying in the io vector from userland. vfs_nmount() first calls vfs_buildopts(). Note there's no check up to this point that the user passing the options in is indeed root. So, all the bugs mentioned can be tickled by a non-root user. vfs_buildopts doesn't ensure that each option's "name" is a null terminated string, but, it later calls vfs_sanitizeopts(), which assumes it is. By passing in strings just at and just over the pagesize, you can cause vfs_buildopts to feed vfs_sanitizeopts() something that makes it vomit quite quickly, by causing strcmp to hit an unmappped page. Another issue: vfs_buildopts also leaks memory if it jumps to "bad": anything in the current option is lost to the woods. It's left as an exercise to the reader to write a program to keep making invalid requests to nmount to exhaust kernel memory. Finally, there's no checking on how much memory is actually aquired by vfs_buildopts(): it can be passed up to MAX_IOVCOUNT (1024) elements in the iovec, and each of these can have up to 64K of data. Unlike passing I/O vectors to writev, this all gets malloc()d in the kernel, so a single call to nmount() by a normal user can get the kernel to allocate over 64M of memory. >How-To-Repeat: #include <sys/param.h> #include <sys/uio.h> #include <sys/mount.h> #include <stdio.h> #include <unistd.h> #include <errno.h> #include <stdlib.h> #include <string.h> static const int OPTCOUNT=128; int main(int argc, char *argv[]) { int pagesize = getpagesize(); char *buf = malloc(pagesize + 1); int i; struct iovec iov[OPTCOUNT * 2]; memset(buf, pagesize + 1, 'x'); /* complete page + 1, not null terminated. */ for (i = 0; i < OPTCOUNT; i++) { iov[i*2].iov_len = pagesize + i % 2; iov[i*2].iov_base = buf; iov[i*2+1].iov_len = 1; iov[i*2+1].iov_base = buf; } if (nmount(iov, i, 0) < 0) fprintf(stderr, "well, we didn't panic\n"); else fprintf(stderr, "success??\n"); } >Fix: I've verified that the patch below solves the problem for me locally. (This is a slightly modified version of the original: A kindly hacker pointed out my C++ism) "MAXOPTMEM" may need to be raised, but for the life of me, I can't see why you'd need to pass a mount system call more than 64k of options. Index: vfs_mount.c =================================================================== RCS file: /pub/FreeBSD/development/FreeBSD-CVS/src/sys/kern/vfs_mount.c,v retrieving revision 1.112 diff -u -r1.112 vfs_mount.c --- vfs_mount.c 5 Nov 2003 04:30:07 -0000 1.112 +++ vfs_mount.c 6 Nov 2003 20:24:13 -0000 @@ -98,6 +98,7 @@ #endif #define ROOTNAME "root_device" +#define MAXOPTMEM (1024 * 64) static void checkdirs(struct vnode *olddp, struct vnode *newdp); static int vfs_nmount(struct thread *td, int, struct uio *); @@ -243,6 +244,7 @@ struct vfsopt *opt; unsigned int i, iovcnt; int error, namelen, optlen; + size_t memtotal = 0; iovcnt = auio->uio_iovcnt; opts = malloc(sizeof(struct vfsoptlist), M_MOUNT, M_WAITOK); @@ -253,6 +255,25 @@ optlen = auio->uio_iov[i + 1].iov_len; opt->name = malloc(namelen, M_MOUNT, M_WAITOK); opt->value = NULL; + opt->len = optlen; + + /* + * Do this early, so jumps to "bad" will free the current + * option + */ + TAILQ_INSERT_TAIL(opts, opt, link); + memtotal += sizeof (struct vfsopt) + optlen + namelen; + + /* + * Avoid consuming too much memory, and attempts to overflow + * memtotal + */ + if (memtotal > MAXOPTMEM || optlen > MAXOPTMEM || + namelen > MAXOPTMEM) { + error = EINVAL; + goto bad; + } + if (auio->uio_segflg == UIO_SYSSPACE) { bcopy(auio->uio_iov[i].iov_base, opt->name, namelen); } else { @@ -261,7 +282,11 @@ if (error) goto bad; } - opt->len = optlen; + /* Ensure names are null-terminated strings */ + if (opt->name[namelen - 1] != '\0') { + error = EINVAL; + goto bad; + } if (optlen != 0) { opt->value = malloc(optlen, M_MOUNT, M_WAITOK); if (auio->uio_segflg == UIO_SYSSPACE) { @@ -274,7 +299,6 @@ goto bad; } } - TAILQ_INSERT_TAIL(opts, opt, link); } vfs_sanitizeopts(opts); *options = opts; >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311111436.hABEaH2p040680>