From dd85b621cf838d5e12e9d58ed60790afb2ec283f Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 11 Jun 2012 15:57:19 +0200 Subject: [PATCH] qemu: Don't overwrite security labels Currently, if qemuProcessStart fail at some point, e.g. because domain being started wants a PCI/USB device already assigned to a different domain, we jump to cleanup label where qemuProcessStop is performed. This unconditionally calls virSecurityManagerRestoreAllLabel which is wrong because the other domain is still using those devices. However, once we successfully label all devices/paths in qemuProcessStart() from that point on, we have to perform a rollback on failure - that is - we have to virSecurityManagerRestoreAllLabel. (cherry picked from commit 86032b2276ace5a7977aad2bbae73b4c33e31914) Conflicts: src/qemu/qemu_process.c --- src/qemu/qemu_process.c | 24 +++++++++++++++++++----- src/qemu/qemu_process.h | 3 ++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 244dcdc948..8cd7abecca 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3281,8 +3281,15 @@ int qemuProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; unsigned long cur_balloon; + unsigned int stop_flags; int i; + /* From now on until domain security labeling is done: + * if any operation fails and we goto cleanup, we must not + * restore any security label as we would overwrite labels + * we did not set. */ + stop_flags = VIR_QEMU_PROCESS_STOP_NO_RELABEL; + hookData.conn = conn; hookData.vm = vm; hookData.driver = driver; @@ -3601,6 +3608,12 @@ int qemuProcessStart(virConnectPtr conn, vm->def, stdin_path) < 0) goto cleanup; + /* Security manager labeled all devices, therefore + * if any operation from now on fails and we goto cleanup, + * where virSecurityManagerRestoreAllLabel() is called + * (hidden under qemuProcessStop) we need to restore labels. */ + stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL; + if (stdin_fd != -1) { /* if there's an fd to migrate from, and it's a pipe, put the * proper security label on it @@ -3738,7 +3751,7 @@ cleanup: * pretend we never started it */ virCommandFree(cmd); VIR_FORCE_CLOSE(logfile); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0); + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags); return -1; } @@ -3951,10 +3964,11 @@ void qemuProcessStop(struct qemud_driver *driver, VIR_FREE(xml); } - /* Reset Security Labels */ - virSecurityManagerRestoreAllLabel(driver->securityManager, - vm->def, - flags & VIR_QEMU_PROCESS_STOP_MIGRATED); + /* Reset Security Labels unless caller don't want us to */ + if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm->def, + flags & VIR_QEMU_PROCESS_STOP_MIGRATED); virSecurityManagerReleaseLabel(driver->securityManager, vm->def); /* Clear out dynamically assigned labels */ diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 4b3a628582..10ba724ddc 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -57,7 +57,8 @@ int qemuProcessStart(virConnectPtr conn, enum virNetDevVPortProfileOp vmop); typedef enum { - VIR_QEMU_PROCESS_STOP_MIGRATED = 1 << 0, + VIR_QEMU_PROCESS_STOP_MIGRATED = 1 << 0, + VIR_QEMU_PROCESS_STOP_NO_RELABEL = 1 << 1, } qemuProcessStopFlags; void qemuProcessStop(struct qemud_driver *driver,