External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-01-03 11:35:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC shutdown/reboot code

Browse Source 
Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
lxcDomainReboot.  Otherwise, a malicious guest could use symlinks
to force the host to manipulate the wrong file in the host's namespace.

Idea by Dan Berrange, based on an initial report by Reco
<recoverym4n@gmail.com> at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=732394

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit aebbcdd33c)

Conflicts:
	src/lxc/lxc_driver.c: OOM error reporting changes
	src/util/virinitctl.c: OOM error reporting changes
This commit is contained in:
Eric Blake 2013-12-23 22:55:51 -07:00 committed by Daniel P. Berrange
parent 1e731bb34d
commit e1e7e05376
3 changed files with 32 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/lxc/lxc_driver.c
View File

@ -2775,6 +2775,15 @@ lxcConnectListAllDomains(virConnectPtr conn,
}
static int
lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED,
void *opaque)
{
int *command = opaque;
return virInitctlSetRunLevel(*command);
}
static int
lxcDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
@ -2782,7 +2791,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
virLXCDriverPtr driver = dom->conn->privateData;
virLXCDomainObjPrivatePtr priv;
virDomainObjPtr vm;
char *vroot = NULL;
int ret = -1;
int rc;
@ -2815,18 +2823,14 @@ lxcDomainShutdownFlags(virDomainPtr dom,
goto cleanup;
}
if (virAsprintf(&vroot, "/proc/%llu/root",
(unsigned long long)priv->initpid) < 0) {
virReportOOMError();
goto cleanup;
}
if (flags == 0 ||
(flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
vroot)) < 0) {
int command = VIR_INITCTL_RUNLEVEL_POWEROFF;
if ((rc = virProcessRunInMountNamespace(priv->initpid,
lxcDomainInitctlCallback,
&command)) < 0)
goto cleanup;
}
if (rc == 0 && flags != 0 &&
((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@ -2852,7 +2856,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
ret = 0;
cleanup:
VIR_FREE(vroot);
if (vm)
virObjectUnlock(vm);
return ret;
@ -2864,6 +2867,7 @@ lxcDomainShutdown(virDomainPtr dom)
return lxcDomainShutdownFlags(dom, 0);
}
static int
lxcDomainReboot(virDomainPtr dom,
unsigned int flags)
@ -2871,7 +2875,6 @@ lxcDomainReboot(virDomainPtr dom,
virLXCDriverPtr driver = dom->conn->privateData;
virLXCDomainObjPrivatePtr priv;
virDomainObjPtr vm;
char *vroot = NULL;
int ret = -1;
int rc;
@ -2904,18 +2907,14 @@ lxcDomainReboot(virDomainPtr dom,
goto cleanup;
}
if (virAsprintf(&vroot, "/proc/%llu/root",
(unsigned long long)priv->initpid) < 0) {
virReportOOMError();
goto cleanup;
}
if (flags == 0 ||
(flags & VIR_DOMAIN_REBOOT_INITCTL)) {
if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
vroot)) < 0) {
int command = VIR_INITCTL_RUNLEVEL_REBOOT;
if ((rc = virProcessRunInMountNamespace(priv->initpid,
lxcDomainInitctlCallback,
&command)) < 0)
goto cleanup;
}
if (rc == 0 && flags != 0 &&
((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@ -2941,7 +2940,6 @@ lxcDomainReboot(virDomainPtr dom,
ret = 0;
cleanup:
VIR_FREE(vroot);
if (vm)
virObjectUnlock(vm);
return ret;

30
src/util/virinitctl.c
View File

@ -110,16 +110,18 @@ struct virInitctlRequest {
# endif
/*
* Send a message to init to change the runlevel
* Send a message to init to change the runlevel. This function is
* asynchronous-signal-safe (thus safe to use after fork of a
* multithreaded parent) - which is good, because it should only be
* used after forking and entering correct namespace.
*
* Returns 1 on success, 0 if initctl does not exist, -1 on error
*/
int virInitctlSetRunLevel(virInitctlRunLevel level,
const char *vroot)
int
virInitctlSetRunLevel(virInitctlRunLevel level)
{
struct virInitctlRequest req;
int fd = -1;
char *path = NULL;
int ret = -1;
memset(&req, 0, sizeof(req));
@ -130,40 +132,28 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
/* Yes it is an 'int' field, but wants a numeric character. Go figure */
req.runlevel = '0' + level;
if (vroot) {
if (virAsprintf(&path, "%s/%s", vroot, VIR_INITCTL_FIFO) < 0) {
virReportOOMError();
return -1;
}
} else {
if (!(path = strdup(VIR_INITCTL_FIFO))) {
virReportOOMError();
return -1;
}
}
if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) < 0) {
if ((fd = open(VIR_INITCTL_FIFO,
O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) < 0) {
if (errno == ENOENT) {
ret = 0;
goto cleanup;
}
virReportSystemError(errno,
_("Cannot open init control %s"),
path);
VIR_INITCTL_FIFO);
goto cleanup;
}
if (safewrite(fd, &req, sizeof(req)) != sizeof(req)) {
virReportSystemError(errno,
_("Failed to send request to init control %s"),
path);
VIR_INITCTL_FIFO);
goto cleanup;
}
ret = 1;
cleanup:
VIR_FREE(path);
VIR_FORCE_CLOSE(fd);
return ret;
}

5
src/util/virinitctl.h
View File

@ -1,7 +1,7 @@
/*
* virinitctl.h: API for talking to init systems via initctl
*
* Copyright (C) 2012 Red Hat, Inc.
* Copyright (C) 2012-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@ -37,7 +37,6 @@ enum virInitctlRunLevel {
VIR_INITCTL_RUNLEVEL_LAST
};
int virInitctlSetRunLevel(virInitctlRunLevel level,
const char *vroot);
int virInitctlSetRunLevel(virInitctlRunLevel level);
#endif