d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add missing privilege check when setting the dump device. Before that change it

Browse Source 
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
This commit is contained in:
Pawel Jakub Dawidek 2014-11-11 04:48:09 +00:00
parent 0b837c87ce
commit 5ebb15b942
4 changed files with 15 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
sys/dev/null/null.c
View File

@ -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;

14
sys/geom/geom_dev.c
View File

@ -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);

7
sys/kern/kern_shutdown.c
View File

@ -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);

2
sys/sys/conf.h
View File

@ -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);