Re: [libvirt] [RFC PATCH v2] qemu: Add suport for pci-assign.configfd option

This allows libvirt to open the PCI device sysfs config file prior to dropping privileges so qemu can access the full config space. Without this, a de-privileged qemu can only access the first 64 bytes of config space. Signed-off-by: Alex Williamson <alex.williamson(a)redhat.com&gt; --- v2: - fix qemuargs, two separate args (thanks Chris) - don't hardcode pci segment - error message of open() fail src/qemu/qemu_conf.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_conf.h | 8 ++++ src/qemu/qemu_driver.c | 2 + 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d7bc798..24c360e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1345,6 +1345,48 @@ fail: return -1; } +void qemudParsePCIDeviceStrs(const char *qemu, unsigned long long *flags)

+{ + const char *const qemuarg[] = { qemu, "-device", "pci-assign,?", NULL }; + const char *const qemuenv[] = { "LC_ALL=C", NULL }; + pid_t child; + int status; + int newstderr = -1; + + if (virExec(qemuarg, qemuenv, NULL, + &child, -1, NULL, &newstderr, VIR_EXEC_CLEAR_CAPS) < 0) + return; + + char *pciassign = NULL; + enum { MAX_PCI_OUTPUT_SIZE = 1024*4 }; + int len = virFileReadLimFD(newstderr, MAX_PCI_OUTPUT_SIZE, &pciassign); + if (len < 0) { + virReportSystemError(errno, + _("Unable to read %s pci-assign device output"), + qemu); + goto cleanup; + } + + if (strstr(pciassign, "pci-assign.configfd")) + *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + +cleanup: + VIR_FREE(pciassign); + close(newstderr); +rewait: + if (waitpid(child, &status, 0) != child) { + if (errno == EINTR) + goto rewait; + + VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"), + WEXITSTATUS(status), (unsigned long)child); + } + if (WEXITSTATUS(status) != 0) { + VIR_WARN("Unexpected exit status '%d', qemu probably failed", + WEXITSTATUS(status)); + } +} + int qemudExtractVersionInfo(const char *qemu, unsigned int *retversion, unsigned long long *retflags) { @@ -1378,6 +1420,9 @@ int qemudExtractVersionInfo(const char *qemu, &version, &is_kvm, &kvm_version) == -1) goto cleanup2; + if (flags & QEMUD_CMD_FLAG_DEVICE) + qemudParsePCIDeviceStrs(qemu, &flags); + if (retversion) *retversion = version; if (retflags) @@ -2887,8 +2932,33 @@ error: } +int +qemudOpenPCIConfig(virDomainHostdevDefPtr dev) +{ + char *path = NULL; + int configfd = -1; + + if (virAsprintf(&path, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/config", + dev->source.subsys.u.pci.domain, + dev->source.subsys.u.pci.bus, + dev->source.subsys.u.pci.slot, + dev->source.subsys.u.pci.function) < 0) { + virReportOOMError(); + return -1; + } + + configfd = open(path, O_RDWR, 0); + + if (configfd < 0) + virReportSystemError(errno, _("Failed opening %s"), path); + + VIR_FREE(path); + + return configfd; +} + char * -qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev) +qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, int configfd) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2898,6 +2968,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev) dev->source.subsys.u.pci.slot, dev->source.subsys.u.pci.function); virBufferVSprintf(&buf, ",id=%s", dev->info.alias); + if (configfd >= 0) + virBufferVSprintf(&buf, ",configfd=%d", configfd);

@@ -4600,8 +4672,21 @@ int qemudBuildCommandLine(virConnectPtr conn, if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + int configfd = -1;

+ if (qemuCmdFlags & QEMUD_CMD_FLAG_PCI_CONFIGFD) { + configfd = qemudOpenPCIConfig(hostdev); + + if (configfd >= 0) {

+ if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { + close(configfd); + goto no_memory; + } + + (*vmfds)[(*nvmfds)++] = configfd; + } + } ADD_ARG_LIT("-device"); - if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev))) + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd)))

goto error; ADD_ARG(devstr); } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a101e47..64fab60 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -88,6 +88,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NO_HPET = (1LL << 33), /* -no-hpet flag is supported */ QEMUD_CMD_FLAG_NO_KVM_PIT = (1LL << 34), /* -no-kvm-pit-reinjection supported */ QEMUD_CMD_FLAG_TDF = (1LL << 35), /* -tdf flag (user-mode pit catchup) */ + QEMUD_CMD_FLAG_PCI_CONFIGFD = (1LL << 36), /* pci-assign.configfd */ }; /* Main driver state */ @@ -190,6 +191,9 @@ int qemudParseHelpStr (const char *qemu, unsigned int *is_kvm, unsigned int *kvm_version); +void qemudParsePCIDeviceStrs (const char *qemu, + unsigned long long *qemuCmdFlags); + int qemudBuildCommandLine (virConnectPtr conn, struct qemud_driver *driver, virDomainDefPtr def, @@ -242,7 +246,9 @@ char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound); /* Legacy, pre device support */ char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev); /* Current, best practice */ -char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev); +char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, int configfd); + +int qemudOpenPCIConfig(virDomainHostdevDefPtr dev); /* Current, best practice */ char * qemuBuildChrChardevStr(virDomainChrDefPtr dev); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dd5bd24..219a973 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7567,7 +7567,7 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0) goto error; - if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev))) + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, -1))) goto error; }