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

diff --git a/sys/dev/null/null.c b/sys/dev/null/null.c
index f836147a773c9b0741fc22f612cc3025055cf986..c8966df8ac4bbaa49308c14a14b0ce85df241630 100644 (file)
--- a/sys/dev/null/null.c
+++ b/sys/dev/null/null.c
@@ -37,7 +37,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/kernel.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
-#include <sys/priv.h>
 #include <sys/disk.h>
 #include <sys/bus.h>
 #include <sys/filio.h>
@@ -110,9 +109,7 @@ null_ioctl(struct cdev *dev __unused, u_long cmd, caddr_t data __unused,
 
        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;

diff --git a/sys/geom/geom_dev.c b/sys/geom/geom_dev.c
index 7cb756befb8f2bdfd9a877bb0a2202d182e42232..6380e407f0e07e28f2b97e6dbf014c708f8a0ac6 100644 (file)
--- a/sys/geom/geom_dev.c
+++ b/sys/geom/geom_dev.c
@@ -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, caddr_t data, int fflag, struct thread
                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);

diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c
index dfdca15194aa6eed188dcfab50ab1f6e223e9f74..357099b59b09a1c918bee0ffa715879aa0c1521e 100644 (file)
--- a/sys/kern/kern_shutdown.c
+++ b/sys/kern/kern_shutdown.c
@@ -827,9 +827,14 @@ SYSCTL_STRING(_kern_shutdown, OID_AUTO, dumpdevname, CTLFLAG_RD,
 
 /* 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);

diff --git a/sys/sys/conf.h b/sys/sys/conf.h
index 8c50581cb1caf6d4f8253186899bdba32f74f0e5..9d73d59a078cbf0a1d67547903ee24efbb47e381 100644 (file)
--- a/sys/sys/conf.h
+++ b/sys/sys/conf.h
@@ -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);