[libvirt] [PATCH 0/5] Fix labeling for evdev passthrough hotplug

https://bugzilla.redhat.com/show_bug.cgi?id=1509866 Ján Tomko (5): Introduce virDomainInputDefGetPath security: Introduce functions for input device hot(un)plug qemu: Introduce functions for input device cgroup manipulation qemu: functions for dealing with input device namespaces and labels qemu: Properly label and create evdev on input device hotplug src/conf/domain_conf.c | 16 +++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 ++ src/qemu/qemu_cgroup.c | 25 ++++++++++- src/qemu/qemu_cgroup.h | 4 ++ src/qemu/qemu_domain.c | 93 +++++++++++++++++++++++++++++++++-------- src/qemu/qemu_domain.h | 6 +++ src/qemu/qemu_hotplug.c | 40 ++++++++++++++++-- src/qemu/qemu_security.c | 58 +++++++++++++++++++++++++ src/qemu/qemu_security.h | 6 +++ src/security/security_dac.c | 3 ++ src/security/security_driver.h | 9 ++++ src/security/security_manager.c | 36 ++++++++++++++++ src/security/security_manager.h | 8 ++++ src/security/security_nop.c | 11 +++++ src/security/security_selinux.c | 3 ++ src/security/security_stack.c | 38 +++++++++++++++++ 17 files changed, 339 insertions(+), 21 deletions(-) -- 2.13.6

Use it to denadify qemuDomainSetupInput. --- src/conf/domain_conf.c | 16 ++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 21 ++++----------------- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 969a6632b..5d0290d07 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1404,6 +1404,22 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) VIR_FREE(def); } +const char *virDomainInputDefGetPath(virDomainInputDefPtr input) +{ + switch ((virDomainInputType) input->type) { + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + case VIR_DOMAIN_INPUT_TYPE_TABLET: + case VIR_DOMAIN_INPUT_TYPE_KBD: + case VIR_DOMAIN_INPUT_TYPE_LAST: + return NULL; + break; + + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + return input->source.evdev; + } + return NULL; +} + void virDomainInputDefFree(virDomainInputDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cb8701dd2..e8ab9abdf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2706,6 +2706,7 @@ int virDomainObjWaitUntil(virDomainObjPtr vm, void virDomainPanicDefFree(virDomainPanicDefPtr panic); void virDomainResourceDefFree(virDomainResourceDefPtr resource); void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); +const char *virDomainInputDefGetPath(virDomainInputDefPtr input); void virDomainInputDefFree(virDomainInputDefPtr def); virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt); void virDomainDiskDefFree(virDomainDiskDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d3ca6b2ec..2997a469d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -395,6 +395,7 @@ virDomainHypervTypeToString; virDomainInputBusTypeToString; virDomainInputDefFind; virDomainInputDefFree; +virDomainInputDefGetPath; virDomainIOMMUModelTypeFromString; virDomainIOMMUModelTypeToString; virDomainIOThreadIDAdd; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc7596bad..b2fc3b816 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8949,25 +8949,12 @@ qemuDomainSetupInput(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainInputDefPtr input, const struct qemuDomainCreateDeviceData *data) { - int ret = -1; - - switch ((virDomainInputType) input->type) { - case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: - if (qemuDomainCreateDevice(input->source.evdev, data, false) < 0) - goto cleanup; - break; + const char *path = virDomainInputDefGetPath(input); - case VIR_DOMAIN_INPUT_TYPE_MOUSE: - case VIR_DOMAIN_INPUT_TYPE_TABLET: - case VIR_DOMAIN_INPUT_TYPE_KBD: - case VIR_DOMAIN_INPUT_TYPE_LAST: - /* nada */ - break; - } + if (path && qemuDomainCreateDevice(path, data, false) < 0) + return -1; - ret = 0; - cleanup: - return ret; + return 0; } -- 2.13.6

Export the existing DAC and SELinux for separate use and introduce functions for stack, nop and the security manager. --- src/libvirt_private.syms | 2 ++ src/security/security_dac.c | 3 +++ src/security/security_driver.h | 9 +++++++++ src/security/security_manager.c | 36 ++++++++++++++++++++++++++++++++++++ src/security/security_manager.h | 8 ++++++++ src/security/security_nop.c | 11 +++++++++++ src/security/security_selinux.c | 3 +++ src/security/security_stack.c | 38 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 110 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2997a469d..31969a092 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1274,6 +1274,7 @@ virSecurityManagerRestoreAllLabel; virSecurityManagerRestoreDiskLabel; virSecurityManagerRestoreHostdevLabel; virSecurityManagerRestoreImageLabel; +virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; virSecurityManagerSetAllLabel; @@ -1283,6 +1284,7 @@ virSecurityManagerSetDiskLabel; virSecurityManagerSetHostdevLabel; virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; +virSecurityManagerSetInputLabel; virSecurityManagerSetMemoryLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 54120890f..52ca07a10 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -2123,6 +2123,9 @@ virSecurityDriver virSecurityDriverDAC = { .domainSetSecurityMemoryLabel = virSecurityDACSetMemoryLabel, .domainRestoreSecurityMemoryLabel = virSecurityDACRestoreMemoryLabel, + .domainSetSecurityInputLabel = virSecurityDACSetInputLabel, + .domainRestoreSecurityInputLabel = virSecurityDACRestoreInputLabel, + .domainSetSecurityDaemonSocketLabel = virSecurityDACSetDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecurityDACSetSocketLabel, .domainClearSecuritySocketLabel = virSecurityDACClearSocketLabel, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 0b3b45248..1b3070d06 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -131,6 +131,12 @@ typedef int (*virSecurityDomainSetMemoryLabel) (virSecurityManagerPtr mgr, typedef int (*virSecurityDomainRestoreMemoryLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainMemoryDefPtr mem); +typedef int (*virSecurityDomainSetInputLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainInputDefPtr input); +typedef int (*virSecurityDomainRestoreInputLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainInputDefPtr input); typedef int (*virSecurityDomainSetPathLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, const char *path); @@ -163,6 +169,9 @@ struct _virSecurityDriver { virSecurityDomainSetMemoryLabel domainSetSecurityMemoryLabel; virSecurityDomainRestoreMemoryLabel domainRestoreSecurityMemoryLabel; + virSecurityDomainSetInputLabel domainSetSecurityInputLabel; + virSecurityDomainRestoreInputLabel domainRestoreSecurityInputLabel; + virSecurityDomainSetDaemonSocketLabel domainSetSecurityDaemonSocketLabel; virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 60cfc92e7..3cf12188a 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1116,3 +1116,39 @@ virSecurityManagerRestoreMemoryLabel(virSecurityManagerPtr mgr, virReportUnsupportedError(); return -1; } + + +int +virSecurityManagerSetInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainInputDefPtr input) +{ + if (mgr->drv->domainSetSecurityInputLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityInputLabel(mgr, vm, input); + virObjectUnlock(mgr); + return ret; + } + + virReportUnsupportedError(); + return -1; +} + + +int +virSecurityManagerRestoreInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainInputDefPtr input) +{ + if (mgr->drv->domainRestoreSecurityInputLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityInputLabel(mgr, vm, input); + virObjectUnlock(mgr); + return ret; + } + + virReportUnsupportedError(); + return -1; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 6712112e7..834c7f159 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -172,6 +172,14 @@ int virSecurityManagerRestoreMemoryLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virDomainMemoryDefPtr mem); +int virSecurityManagerSetInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainInputDefPtr input); +int virSecurityManagerRestoreInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainInputDefPtr input); + + int virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *path); diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 527be11e5..cfb032c68 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -254,6 +254,14 @@ virSecurityDomainRestoreMemoryLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSE return 0; } +static int +virSecurityDomainInputLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainInputDefPtr input ATTRIBUTE_UNUSED) +{ + return 0; +} + virSecurityDriver virSecurityDriverNop = { .privateDataLen = 0, @@ -276,6 +284,9 @@ virSecurityDriver virSecurityDriverNop = { .domainSetSecurityMemoryLabel = virSecurityDomainSetMemoryLabelNop, .domainRestoreSecurityMemoryLabel = virSecurityDomainRestoreMemoryLabelNop, + .domainSetSecurityInputLabel = virSecurityDomainInputLabelNop, + .domainRestoreSecurityInputLabel = virSecurityDomainInputLabelNop, + .domainSetSecurityDaemonSocketLabel = virSecurityDomainSetDaemonSocketLabelNop, .domainSetSecuritySocketLabel = virSecurityDomainSetSocketLabelNop, .domainClearSecuritySocketLabel = virSecurityDomainClearSocketLabelNop, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index ed1828a12..b677fbcda 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3064,6 +3064,9 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSetSecurityMemoryLabel = virSecuritySELinuxSetMemoryLabel, .domainRestoreSecurityMemoryLabel = virSecuritySELinuxRestoreMemoryLabel, + .domainSetSecurityInputLabel = virSecuritySELinuxSetInputLabel, + .domainRestoreSecurityInputLabel = virSecuritySELinuxRestoreInputLabel, + .domainSetSecurityDaemonSocketLabel = virSecuritySELinuxSetDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecuritySELinuxSetSocketLabel, .domainClearSecuritySocketLabel = virSecuritySELinuxClearSocketLabel, diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 53eee1692..cd916382b 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -667,6 +667,41 @@ virSecurityStackRestoreMemoryLabel(virSecurityManagerPtr mgr, } static int +virSecurityStackSetInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainInputDefPtr input) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetInputLabel(item->securityManager, vm, input) < 0) + rc = -1; + } + + return rc; +} + +static int +virSecurityStackRestoreInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainInputDefPtr input) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerRestoreInputLabel(item->securityManager, + vm, input) < 0) + rc = -1; + } + + return rc; +} + +static int virSecurityStackDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *path) @@ -711,6 +746,9 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityMemoryLabel = virSecurityStackSetMemoryLabel, .domainRestoreSecurityMemoryLabel = virSecurityStackRestoreMemoryLabel, + .domainSetSecurityInputLabel = virSecurityStackSetInputLabel, + .domainRestoreSecurityInputLabel = virSecurityStackRestoreInputLabel, + .domainSetSecurityDaemonSocketLabel = virSecurityStackSetDaemonSocketLabel, .domainSetSecuritySocketLabel = virSecurityStackSetSocketLabel, .domainClearSecuritySocketLabel = virSecurityStackClearSocketLabel, -- 2.13.6

Export qemuSetupInputCgroup and introduce qemuTeardownInputCgroup for hotunplug. --- src/qemu/qemu_cgroup.c | 25 ++++++++++++++++++++++++- src/qemu/qemu_cgroup.h | 4 ++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 0f75e22f9..19252ea23 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -246,7 +246,7 @@ qemuSetupTPMCgroup(virDomainObjPtr vm) } -static int +int qemuSetupInputCgroup(virDomainObjPtr vm, virDomainInputDefPtr dev) { @@ -270,6 +270,29 @@ qemuSetupInputCgroup(virDomainObjPtr vm, int +qemuTeardownInputCgroup(virDomainObjPtr vm, + virDomainInputDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = 0; + + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + + switch (dev->type) { + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + VIR_DEBUG("Process path '%s' for input device", dev->source.evdev); + ret = virCgroupDenyDevicePath(priv->cgroup, dev->source.evdev, + VIR_CGROUP_DEVICE_RWM, false); + virDomainAuditCgroupPath(vm, priv->cgroup, "deny", dev->source.evdev, "rwm", ret == 0); + break; + } + + return ret; +} + + +int qemuSetupHostdevCgroup(virDomainObjPtr vm, virDomainHostdevDefPtr dev) { diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 3fc158361..3b8ff6055 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -37,6 +37,10 @@ int qemuSetupDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk); int qemuTeardownDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk); +int qemuSetupInputCgroup(virDomainObjPtr vm, + virDomainInputDefPtr dev); +int qemuTeardownInputCgroup(virDomainObjPtr vm, + virDomainInputDefPtr dev); int qemuSetupHostdevCgroup(virDomainObjPtr vm, virDomainHostdevDefPtr dev) ATTRIBUTE_RETURN_CHECK; -- 2.13.6

Introudce functions that will let us create the evdevs in namespaces and label the devices on input device hotplug/hotunplug. --- src/qemu/qemu_domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 6 ++++ src/qemu/qemu_security.c | 58 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_security.h | 6 ++++ 4 files changed, 142 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b2fc3b816..5831a2025 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9969,6 +9969,78 @@ qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver, } +int +qemuDomainNamespaceSetupInput(virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virQEMUDriverConfigPtr cfg = NULL; + char **devMountsPath = NULL; + size_t ndevMountsPath = 0; + const char *path = NULL; + int ret = -1; + + if (!(path = virDomainInputDefGetPath(input))) + return 0; + + if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) + return 0; + + cfg = virQEMUDriverGetConfig(driver); + if (qemuDomainGetPreservedMounts(cfg, vm, + &devMountsPath, NULL, + &ndevMountsPath) < 0) + goto cleanup; + + if (qemuDomainAttachDeviceMknod(driver, vm, path, + devMountsPath, ndevMountsPath) < 0) + goto cleanup; + + ret = 0; + cleanup: + virStringListFreeCount(devMountsPath, ndevMountsPath); + virObjectUnref(cfg); + return ret; +} + + +int +qemuDomainNamespaceTeardownInput(virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virQEMUDriverConfigPtr cfg = NULL; + char **devMountsPath = NULL; + size_t ndevMountsPath = 0; + const char *path = NULL; + int ret = -1; + + if (!(path = virDomainInputDefGetPath(input))) + return 0; + + if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) + return 0; + + cfg = virQEMUDriverGetConfig(driver); + if (qemuDomainGetPreservedMounts(cfg, vm, + &devMountsPath, NULL, + &ndevMountsPath) < 0) + goto cleanup; + + if (qemuDomainDetachDeviceUnlink(driver, vm, path, + devMountsPath, ndevMountsPath) < 0) + goto cleanup; + + ret = 0; + cleanup: + virStringListFreeCount(devMountsPath, ndevMountsPath); + virObjectUnref(cfg); + return ret; +} + + /** * qemuDomainDiskLookupByNodename: * @def: domain definition to look for the disk diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e021da51f..e699ab5ba 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -968,6 +968,12 @@ int qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainRNGDefPtr rng); +int qemuDomainNamespaceSetupInput(virDomainObjPtr vm, + virDomainInputDefPtr input); + +int qemuDomainNamespaceTeardownInput(virDomainObjPtr vm, + virDomainInputDefPtr input); + virDomainDiskDefPtr qemuDomainDiskLookupByNodename(virDomainDefPtr def, const char *nodename, virStorageSourcePtr *src, diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 6fc3b0bb6..e7d2bbd5a 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -306,3 +306,61 @@ qemuSecurityRestoreMemoryLabel(virQEMUDriverPtr driver, virSecurityManagerTransactionAbort(driver->securityManager); return ret; } + + +int +qemuSecuritySetInputLabel(virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + int ret = -1; + + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) && + virSecurityManagerTransactionStart(driver->securityManager) < 0) + goto cleanup; + + if (virSecurityManagerSetInputLabel(driver->securityManager, + vm->def, + input) < 0) + goto cleanup; + + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) && + virSecurityManagerTransactionCommit(driver->securityManager, + vm->pid) < 0) + goto cleanup; + + ret = 0; + cleanup: + virSecurityManagerTransactionAbort(driver->securityManager); + return ret; +} + + +int +qemuSecurityRestoreInputLabel(virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + int ret = -1; + + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) && + virSecurityManagerTransactionStart(driver->securityManager) < 0) + goto cleanup; + + if (virSecurityManagerRestoreInputLabel(driver->securityManager, + vm->def, + input) < 0) + goto cleanup; + + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) && + virSecurityManagerTransactionCommit(driver->securityManager, + vm->pid) < 0) + goto cleanup; + + ret = 0; + cleanup: + virSecurityManagerTransactionAbort(driver->securityManager); + return ret; +} diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 7b25855bf..76d63f06e 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -70,6 +70,12 @@ int qemuSecurityRestoreMemoryLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainMemoryDefPtr mem); +int qemuSecuritySetInputLabel(virDomainObjPtr vm, + virDomainInputDefPtr input); + +int qemuSecurityRestoreInputLabel(virDomainObjPtr vm, + virDomainInputDefPtr input); + /* Please note that for these APIs there is no wrapper yet. Do NOT blindly add * new APIs here. If an API can touch a /dev file add a proper wrapper instead. */ -- 2.13.6

On 11/21/2017 04:05 PM, Ján Tomko wrote:
Introudce functions that will let us create the evdevs in namespaces and label the devices on input device hotplug/hotunplug. --- src/qemu/qemu_domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 6 ++++ src/qemu/qemu_security.c | 58 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_security.h | 6 ++++ 4 files changed, 142 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b2fc3b816..5831a2025 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9969,6 +9969,78 @@ qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver, }
+int +qemuDomainNamespaceSetupInput(virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virQEMUDriverConfigPtr cfg = NULL; + char **devMountsPath = NULL; + size_t ndevMountsPath = 0; + const char *path = NULL; + int ret = -1; + + if (!(path = virDomainInputDefGetPath(input))) + return 0; + + if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) + return 0;
Just a small nit. I prefer this namespace check to be the first in the function (look at qemuDomainNamespaceSetupChardev() for instance).
+ + cfg = virQEMUDriverGetConfig(driver); + if (qemuDomainGetPreservedMounts(cfg, vm, + &devMountsPath, NULL, + &ndevMountsPath) < 0) + goto cleanup; + + if (qemuDomainAttachDeviceMknod(driver, vm, path, + devMountsPath, ndevMountsPath) < 0) + goto cleanup; + + ret = 0; + cleanup: + virStringListFreeCount(devMountsPath, ndevMountsPath); + virObjectUnref(cfg); + return ret; +} + + +int +qemuDomainNamespaceTeardownInput(virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virQEMUDriverConfigPtr cfg = NULL; + char **devMountsPath = NULL; + size_t ndevMountsPath = 0; + const char *path = NULL; + int ret = -1; + + if (!(path = virDomainInputDefGetPath(input))) + return 0; + + if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) + return 0;
Here too.
+ + cfg = virQEMUDriverGetConfig(driver); + if (qemuDomainGetPreservedMounts(cfg, vm, + &devMountsPath, NULL, + &ndevMountsPath) < 0) + goto cleanup; + + if (qemuDomainDetachDeviceUnlink(driver, vm, path, + devMountsPath, ndevMountsPath) < 0) + goto cleanup; + + ret = 0; + cleanup: + virStringListFreeCount(devMountsPath, ndevMountsPath); + virObjectUnref(cfg); + return ret; +} +
Michal

Utilize all the newly introduced function to create the evdev node and label it on hotplug and destroy it on hotunplug. This was forgotten in commits bc9ffaf and 67486bb. https://bugzilla.redhat.com/show_bug.cgi?id=1509866 --- src/qemu/qemu_hotplug.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 72a57d89e..fe69d42e8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2746,7 +2746,11 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_INPUT, { .input = input } }; + virErrorPtr originalError = NULL; bool releaseaddr = false; + bool teardowndevice = false; + bool teardownlabel = false; + bool teardowncgroup = false; if (input->bus != VIR_DOMAIN_INPUT_BUS_USB && input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO) { @@ -2773,6 +2777,18 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver, if (qemuBuildInputDevStr(&devstr, vm->def, input, priv->qemuCaps) < 0) goto cleanup; + if (qemuDomainNamespaceSetupInput(vm, input) < 0) + goto cleanup; + teardowndevice = true; + + if (qemuSetupInputCgroup(vm, input) < 0) + goto cleanup; + teardowncgroup = true; + + if (qemuSecuritySetInputLabel(vm, input) < 0) + goto cleanup; + teardownlabel = true; + if (VIR_REALLOC_N(vm->def->inputs, vm->def->ninputs + 1) < 0) goto cleanup; @@ -2788,14 +2804,23 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver, VIR_APPEND_ELEMENT_COPY_INPLACE(vm->def->inputs, vm->def->ninputs, input); ret = 0; - releaseaddr = false; audit: virDomainAuditInput(vm, input, "attach", ret == 0); cleanup: - if (releaseaddr) - qemuDomainReleaseDeviceAddress(vm, &input->info, NULL); + if (ret < 0) { + virErrorPreserveLast(&originalError); + if (teardownlabel) + qemuSecurityRestoreInputLabel(vm, input); + if (teardowncgroup) + qemuTeardownInputCgroup(vm, input); + if (teardowndevice) + qemuDomainNamespaceTeardownInput(vm, input); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &input->info, NULL); + virErrorRestore(&originalError); + } VIR_FREE(devstr); return ret; @@ -4283,6 +4308,15 @@ qemuDomainRemoveInputDevice(virDomainObjPtr vm, break; } qemuDomainReleaseDeviceAddress(vm, &dev->info, NULL); + if (qemuSecurityRestoreInputLabel(vm, dev) < 0) + VIR_WARN("Unable to restore security label on input device"); + + if (qemuTeardownInputCgroup(vm, dev) < 0) + VIR_WARN("Unable to remove input device cgroup ACL"); + + if (qemuDomainNamespaceTeardownInput(vm, dev) < 0) + VIR_WARN("Unable to remove input device from /dev"); + virDomainInputDefFree(vm->def->inputs[i]); VIR_DELETE_ELEMENT(vm->def->inputs, i, vm->def->ninputs); return 0; -- 2.13.6

On 11/21/2017 04:05 PM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1509866
Ján Tomko (5): Introduce virDomainInputDefGetPath security: Introduce functions for input device hot(un)plug qemu: Introduce functions for input device cgroup manipulation qemu: functions for dealing with input device namespaces and labels qemu: Properly label and create evdev on input device hotplug
src/conf/domain_conf.c | 16 +++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 ++ src/qemu/qemu_cgroup.c | 25 ++++++++++- src/qemu/qemu_cgroup.h | 4 ++ src/qemu/qemu_domain.c | 93 +++++++++++++++++++++++++++++++++-------- src/qemu/qemu_domain.h | 6 +++ src/qemu/qemu_hotplug.c | 40 ++++++++++++++++-- src/qemu/qemu_security.c | 58 +++++++++++++++++++++++++ src/qemu/qemu_security.h | 6 +++ src/security/security_dac.c | 3 ++ src/security/security_driver.h | 9 ++++ src/security/security_manager.c | 36 ++++++++++++++++ src/security/security_manager.h | 8 ++++ src/security/security_nop.c | 11 +++++ src/security/security_selinux.c | 3 ++ src/security/security_stack.c | 38 +++++++++++++++++ 17 files changed, 339 insertions(+), 21 deletions(-)
ACK series. Michal
participants (2)
-
Ján Tomko
-
Michal Privoznik