From owner-svn-src-head@FreeBSD.ORG Tue Nov 11 04:48:11 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5F92A24F; Tue, 11 Nov 2014 04:48:11 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4B9E2F26; Tue, 11 Nov 2014 04:48:11 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id sAB4mBEt022304; Tue, 11 Nov 2014 04:48:11 GMT (envelope-from pjd@FreeBSD.org) Received: (from pjd@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id sAB4mABC022300; Tue, 11 Nov 2014 04:48:10 GMT (envelope-from pjd@FreeBSD.org) Message-Id: <201411110448.sAB4mABC022300@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: pjd set sender to pjd@FreeBSD.org using -f From: Pawel Jakub Dawidek Date: Tue, 11 Nov 2014 04:48:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r274366 - in head/sys: dev/null geom kern sys X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Nov 2014 04:48:11 -0000 Author: pjd Date: Tue Nov 11 04:48:09 2014 New Revision: 274366 URL: https://svnweb.freebsd.org/changeset/base/274366 Log: Add missing privilege check when setting the dump device. Before that change it was possible for a regular user to setup the dump device if he had write access to the given device. In theory it is a security issue as user might get access to kernel's memory after provoking kernel crash, but in practise it is not recommended to give regular users direct access to storage devices. Rework the code so that we do privileges check within the set_dumper() function to avoid similar problems in the future. Discussed with: secteam Modified: head/sys/dev/null/null.c head/sys/geom/geom_dev.c head/sys/kern/kern_shutdown.c head/sys/sys/conf.h Modified: head/sys/dev/null/null.c ============================================================================== --- head/sys/dev/null/null.c Tue Nov 11 04:07:41 2014 (r274365) +++ head/sys/dev/null/null.c Tue Nov 11 04:48:09 2014 (r274366) @@ -37,7 +37,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -110,9 +109,7 @@ null_ioctl(struct cdev *dev __unused, u_ switch (cmd) { case DIOCSKERNELDUMP: - error = priv_check(td, PRIV_SETDUMPER); - if (error == 0) - error = set_dumper(NULL, NULL); + error = set_dumper(NULL, NULL, td); break; case FIONBIO: break; Modified: head/sys/geom/geom_dev.c ============================================================================== --- head/sys/geom/geom_dev.c Tue Nov 11 04:07:41 2014 (r274365) +++ head/sys/geom/geom_dev.c Tue Nov 11 04:48:09 2014 (r274366) @@ -127,14 +127,14 @@ g_dev_fini(struct g_class *mp) } static int -g_dev_setdumpdev(struct cdev *dev) +g_dev_setdumpdev(struct cdev *dev, struct thread *td) { struct g_kerneldump kd; struct g_consumer *cp; int error, len; if (dev == NULL) - return (set_dumper(NULL, NULL)); + return (set_dumper(NULL, NULL, td)); cp = dev->si_drv2; len = sizeof(kd); @@ -142,7 +142,7 @@ g_dev_setdumpdev(struct cdev *dev) kd.length = OFF_MAX; error = g_io_getattr("GEOM::kerneldump", cp, &len, &kd); if (error == 0) { - error = set_dumper(&kd.di, devtoname(dev)); + error = set_dumper(&kd.di, devtoname(dev), td); if (error == 0) dev->si_flags |= SI_DUMPDEV; } @@ -157,7 +157,7 @@ init_dumpdev(struct cdev *dev) return; if (strcmp(devtoname(dev), dumpdev) != 0) return; - if (g_dev_setdumpdev(dev) == 0) { + if (g_dev_setdumpdev(dev, curthread) == 0) { freeenv(dumpdev); dumpdev = NULL; } @@ -453,9 +453,9 @@ g_dev_ioctl(struct cdev *dev, u_long cmd break; case DIOCSKERNELDUMP: if (*(u_int *)data == 0) - error = g_dev_setdumpdev(NULL); + error = g_dev_setdumpdev(NULL, td); else - error = g_dev_setdumpdev(dev); + error = g_dev_setdumpdev(dev, td); break; case DIOCGFLUSH: error = g_io_flush(cp); @@ -673,7 +673,7 @@ g_dev_orphan(struct g_consumer *cp) /* Reset any dump-area set on this device */ if (dev->si_flags & SI_DUMPDEV) - (void)set_dumper(NULL, NULL); + (void)set_dumper(NULL, NULL, curthread); /* Destroy the struct cdev *so we get no more requests */ destroy_dev_sched_cb(dev, g_dev_callback, cp); Modified: head/sys/kern/kern_shutdown.c ============================================================================== --- head/sys/kern/kern_shutdown.c Tue Nov 11 04:07:41 2014 (r274365) +++ head/sys/kern/kern_shutdown.c Tue Nov 11 04:48:09 2014 (r274366) @@ -827,9 +827,14 @@ SYSCTL_STRING(_kern_shutdown, OID_AUTO, /* Registration of dumpers */ int -set_dumper(struct dumperinfo *di, const char *devname) +set_dumper(struct dumperinfo *di, const char *devname, struct thread *td) { size_t wantcopy; + int error; + + error = priv_check(td, PRIV_SETDUMPER); + if (error != 0) + return (error); if (di == NULL) { bzero(&dumper, sizeof dumper); Modified: head/sys/sys/conf.h ============================================================================== --- head/sys/sys/conf.h Tue Nov 11 04:07:41 2014 (r274365) +++ head/sys/sys/conf.h Tue Nov 11 04:48:09 2014 (r274366) @@ -336,7 +336,7 @@ struct dumperinfo { off_t mediasize; /* Space available in bytes. */ }; -int set_dumper(struct dumperinfo *, const char *_devname); +int set_dumper(struct dumperinfo *, const char *_devname, struct thread *td); int dump_write(struct dumperinfo *, void *, vm_offset_t, off_t, size_t); int dumpsys(struct dumperinfo *); int doadump(boolean_t);