[PATCH v2 0/2] qemu: Move 2 pid files to stateDir
This is v2 of: https://listman.redhat.com/archives/libvir-list/2021-October/msg00535.html v1 -> v2: - Fix compatibility with old version libvirt [Michal] Peng Liang (2): qemu: Move pid file of pr-helper to stateDir qemu: Move pid file of virtiofsd to stateDir src/qemu/qemu_process.c | 22 +++++++++++++++++++++- src/qemu/qemu_virtiofs.c | 25 ++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) -- 2.31.1
Libvirt will put the pid file of pr-helper to per-domain directory. However, the ownership of the per-domain directory is the user to run the QEMU process and the user has the write permission of the directory. If VM escape occurs, the attacker can 1. write arbitrary content to the pid file (if running QEMU using root), then the attacker can kill any process by writing appropriate pid to the pid file; 2. spoof the pid file (if running QEMU using a regular user), then the pr-helper process will never be cleared even if the VM is destroyed. So, move the pid file of pr-helper from per-domain directory to stateDir. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/qemu/qemu_process.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f95ed80fac43..6027b30405dc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2856,7 +2856,7 @@ qemuProcessResctrlCreate(virQEMUDriver *driver, static char * -qemuProcessBuildPRHelperPidfilePath(virDomainObj *vm) +qemuProcessBuildPRHelperPidfilePathOld(virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; const char *prdAlias = qemuDomainGetManagedPRAlias(); @@ -2865,6 +2865,18 @@ qemuProcessBuildPRHelperPidfilePath(virDomainObj *vm) } +static char * +qemuProcessBuildPRHelperPidfilePath(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + g_autofree char *domname = virDomainDefGetShortName(vm->def); + g_autofree char *prdName = g_strdup_printf("%s-%s", domname, qemuDomainGetManagedPRAlias()); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); + + return virPidFileBuildPath(cfg->stateDir, prdName); +} + + void qemuProcessKillManagedPRDaemon(virDomainObj *vm) { @@ -2877,6 +2889,14 @@ qemuProcessKillManagedPRDaemon(virDomainObj *vm) return; } + if (!virFileExists(pidfile)) { + g_free(pidfile); + if (!(pidfile = qemuProcessBuildPRHelperPidfilePathOld(vm))) { + VIR_WARN("Unable to construct pr-helper pidfile path"); + return; + } + } + virErrorPreserveLast(&orig_err); if (virPidFileForceCleanupPath(pidfile) < 0) { VIR_WARN("Unable to kill pr-helper process"); -- 2.31.1
Libvirt will put the pid file of virtiofsd to per-domain directory. However, the ownership of the per-domain directory is the user to run the QEMU process and the user has the write permission of the directory. If VM escape occurs, the attacker can 1. write arbitrary content to the pid file (if running QEMU using root), then the attacker can kill any process by writing appropriate pid to the pid file; 2. spoof the pid file (if running QEMU using a regular user), then the virtiofsd process will never be cleared even if the VM is destroyed. So, move the pid file of virtiofsd from per-domain directory to stateDir. Signed-off-by: Peng Liang <liangpeng10@huawei.com> --- src/qemu/qemu_virtiofs.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 3ca45457c16e..0c12c5ea22a6 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -39,9 +39,9 @@ VIR_LOG_INIT("qemu.virtiofs"); -char * -qemuVirtioFSCreatePidFilename(virDomainObj *vm, - const char *alias) +static char * +qemuVirtioFSCreatePidFilenameOld(virDomainObj *vm, + const char *alias) { qemuDomainObjPrivate *priv = vm->privateData; g_autofree char *name = NULL; @@ -52,6 +52,19 @@ qemuVirtioFSCreatePidFilename(virDomainObj *vm, } +char * +qemuVirtioFSCreatePidFilename(virDomainObj *vm, + const char *alias) +{ + qemuDomainObjPrivate *priv = vm->privateData; + g_autofree char *domname = virDomainDefGetShortName(vm->def); + g_autofree char *name = g_strdup_printf("%s-%s-fs", domname, alias); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); + + return virPidFileBuildPath(cfg->stateDir, name); +} + + char * qemuVirtioFSCreateSocketFilename(virDomainObj *vm, const char *alias) @@ -283,6 +296,12 @@ qemuVirtioFSStop(virQEMUDriver *driver G_GNUC_UNUSED, if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias))) goto cleanup; + if (!virFileExists(pidfile)) { + g_free(pidfile); + if (!(pidfile = qemuVirtioFSCreatePidFilenameOld(vm, fs->info.alias))) + goto cleanup; + } + if (virPidFileForceCleanupPathFull(pidfile, true) < 0) { VIR_WARN("Unable to kill virtiofsd process"); } else { -- 2.31.1
On 10/18/21 11:20 AM, Peng Liang wrote:
This is v2 of: https://listman.redhat.com/archives/libvir-list/2021-October/msg00535.html
v1 -> v2: - Fix compatibility with old version libvirt [Michal]
Peng Liang (2): qemu: Move pid file of pr-helper to stateDir qemu: Move pid file of virtiofsd to stateDir
src/qemu/qemu_process.c | 22 +++++++++++++++++++++- src/qemu/qemu_virtiofs.c | 25 ++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peng Liang