The internal representation of security labels contains some bools (e.g. @relabel, @implicit). But the stdbool.h file is not included anywhere in the header file, therefore if somebody just includes "virseclabel.h" they also need to include <stdbool.h>. That's not right as it should be virseclabel.h who drags in all the dependencies. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/util/virseclabel.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h index 94c4dfc..558d4eb 100644 --- a/src/util/virseclabel.h +++ b/src/util/virseclabel.h @@ -22,6 +22,8 @@ #ifndef __SECLABEL_H # define __SECLABEL_H +# include <stdbool.h> + typedef enum { VIR_DOMAIN_SECLABEL_DEFAULT, VIR_DOMAIN_SECLABEL_NONE, -- 2.8.4

This change alone is not needed, but it prepares environment for subsequent patches where we will need virSecurityManager much deeper in the code. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/security/security_dac.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index fd74e8b..97be862 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -349,12 +349,13 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv, static int -virSecurityDACSetOwnership(virSecurityDACDataPtr priv, +virSecurityDACSetOwnership(virSecurityManagerPtr mgr, virStorageSourcePtr src, const char *path, uid_t uid, gid_t gid) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); struct stat sb; if (!path && src && src->path && @@ -442,7 +443,7 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr, return -1; } - return virSecurityDACSetOwnership(priv, src, NULL, user, group); + return virSecurityDACSetOwnership(mgr, src, NULL, user, group); } @@ -550,7 +551,7 @@ virSecurityDACSetHostdevLabelHelper(const char *file, if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL) < 0) return -1; - return virSecurityDACSetOwnership(priv, NULL, file, user, group); + return virSecurityDACSetOwnership(mgr, NULL, file, user, group); } @@ -850,7 +851,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, switch ((virDomainChrType) dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(priv, NULL, + ret = virSecurityDACSetOwnership(mgr, NULL, dev_source->data.file.path, user, group); break; @@ -860,10 +861,10 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0) goto done; if (virFileExists(in) && virFileExists(out)) { - if (virSecurityDACSetOwnership(priv, NULL, in, user, group) < 0 || - virSecurityDACSetOwnership(priv, NULL, out, user, group) < 0) + if (virSecurityDACSetOwnership(mgr, NULL, in, user, group) < 0 || + virSecurityDACSetOwnership(mgr, NULL, out, user, group) < 0) goto done; - } else if (virSecurityDACSetOwnership(priv, NULL, + } else if (virSecurityDACSetOwnership(mgr, NULL, dev_source->data.file.path, user, group) < 0) { goto done; @@ -873,7 +874,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_CHR_TYPE_UNIX: if (!dev_source->data.nix.listen) { - if (virSecurityDACSetOwnership(priv, NULL, + if (virSecurityDACSetOwnership(mgr, NULL, dev_source->data.nix.path, user, group) < 0) goto done; @@ -1033,7 +1034,7 @@ virSecurityDACSetInputLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) return -1; - ret = virSecurityDACSetOwnership(priv, NULL, input->source.evdev, user, group); + ret = virSecurityDACSetOwnership(mgr, NULL, input->source.evdev, user, group); break; case VIR_DOMAIN_INPUT_TYPE_MOUSE: @@ -1199,27 +1200,27 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, return -1; if (def->os.loader && def->os.loader->nvram && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.loader->nvram, user, group) < 0) return -1; if (def->os.kernel && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.kernel, user, group) < 0) return -1; if (def->os.initrd && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.initrd, user, group) < 0) return -1; if (def->os.dtb && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.dtb, user, group) < 0) return -1; if (def->os.slic_table && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.slic_table, user, group) < 0) return -1; @@ -1242,7 +1243,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetImageIds(secdef, priv, &user, &group) < 0) return -1; - return virSecurityDACSetOwnership(priv, NULL, savefile, user, group); + return virSecurityDACSetOwnership(mgr, NULL, savefile, user, group); } @@ -1561,7 +1562,7 @@ virSecurityDACDomainSetPathLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) return -1; - return virSecurityDACSetOwnership(priv, NULL, path, user, group); + return virSecurityDACSetOwnership(mgr, NULL, path, user, group); } virSecurityDriver virSecurityDriverDAC = { -- 2.8.4

This is new internal class that is going to remember <device,[array of seclabels]> pairs. Moreover, it is going to be able to flush the pairs into a file so that a helper (which is introduced later in the series) can look into the file and answer question: "Is this path in use by libvirt and if so what security labels should it have?" You can say that we already have security drivers for that. And you would be right. But unfortunately on a Linux system, some processes running in it reset security labels sometimes, possibly cutting of a running domain. For instance udev. There has been a problem (race you can say), where libvirt set seclabels on a disk device, and wanted to start a domain but meanwhile udev came and restored the seclabels. With this module we can have a small helper that could be used by udev to find out what seclabels should a device have. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/Makefile.am | 1 + src/libvirt_private.syms | 4 +++ src/util/virudev.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virudev.h | 31 ++++++++++++++++++++++ 4 files changed, 104 insertions(+) create mode 100644 src/util/virudev.c create mode 100644 src/util/virudev.h diff --git a/src/Makefile.am b/src/Makefile.am index 8ee5567..2ea6f2b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -179,6 +179,7 @@ UTIL_SOURCES = \ util/virtime.h util/virtime.c \ util/virtpm.h util/virtpm.c \ util/virtypedparam.c util/virtypedparam.h \ + util/virudev.c util/virudev.h \ util/virusb.c util/virusb.h \ util/viruri.h util/viruri.c \ util/virutil.c util/virutil.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd1462b..40c5d27 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2577,6 +2577,10 @@ virTypedParamsSerialize; virTypedParamsValidate; +# util/virudev.h +virUdevMgrNew; + + # util/viruri.h virURIFormat; virURIFormatParams; diff --git a/src/util/virudev.c b/src/util/virudev.c new file mode 100644 index 0000000..66b5a58 --- /dev/null +++ b/src/util/virudev.c @@ -0,0 +1,68 @@ +/* + * virudev.c: udev rules engine + * + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "virudev.h" +#include "virobject.h" + +struct _virUdevMgr { + virObjectLockable parent; +}; + +static virClassPtr virUdevMgrClass; + + +static void +virUdevMgrDispose(void *obj ATTRIBUTE_UNUSED) +{ + /* nada */ +} + + +static int virUdevMgrOnceInit(void) +{ + if (!(virUdevMgrClass = virClassNew(virClassForObjectLockable(), + "virUdevMgr", + sizeof(virUdevMgr), + virUdevMgrDispose))) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virUdevMgr) + + +virUdevMgrPtr virUdevMgrNew(void) +{ + virUdevMgrPtr mgr; + + if (virUdevMgrInitialize() < 0) + return NULL; + + if (!(mgr = virObjectLockableNew(virUdevMgrClass))) + return NULL; + + return mgr; +} diff --git a/src/util/virudev.h b/src/util/virudev.h new file mode 100644 index 0000000..28e336f --- /dev/null +++ b/src/util/virudev.h @@ -0,0 +1,31 @@ +/* + * virudev.h: udev rules engine + * + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#ifndef __VIR_UDEV_H__ +# define __VIR_UDEV_H__ + +typedef struct _virUdevMgr virUdevMgr; +typedef virUdevMgr *virUdevMgrPtr; + +virUdevMgrPtr virUdevMgrNew(void); + +#endif -- 2.8.4

Now that we are able to store security labels for devices, next step is to flush them into a file. For more convenience I've chosen JSON format (as we have all the APIs needed for processing the format). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/libvirt_private.syms | 2 + src/util/virudev.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virudev.h | 5 ++ 4 files changed, 165 insertions(+) diff --git a/po/POTFILES.in b/po/POTFILES.in index 1469240..dabc612 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -248,6 +248,7 @@ src/util/virthreadpool.c src/util/virtime.c src/util/virtpm.c src/util/virtypedparam.c +src/util/virudev.c src/util/viruri.c src/util/virusb.c src/util/virutil.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 073b00f..ca64c80 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2579,6 +2579,8 @@ virTypedParamsValidate; # util/virudev.h virUdevMgrAddLabel; +virUdevMgrDumpFile; +virUdevMgrDumpStr; virUdevMgrNew; virUdevMgrRemoveAllLabels; diff --git a/src/util/virudev.c b/src/util/virudev.c index f4799e7..7f52149 100644 --- a/src/util/virudev.c +++ b/src/util/virudev.c @@ -24,7 +24,9 @@ #include "internal.h" #include "viralloc.h" +#include "virfile.h" #include "virhash.h" +#include "virjson.h" #include "virobject.h" #include "virudev.h" @@ -112,6 +114,68 @@ udevSeclabelUpdate(udevSeclabelPtr list, } +static virJSONValuePtr +udevSeclabelDump(const virSecurityDeviceLabelDef *seclabel) +{ + virJSONValuePtr object; + + if (!(object = virJSONValueNewObject()) || + virJSONValueObjectAppendString(object, "model", seclabel->model) < 0 || + virJSONValueObjectAppendString(object, "label", seclabel->label) < 0) + goto error; + + return object; + + error: + virJSONValueFree(object); + return NULL; +} + + +static int +udevSeclabelsDump(void *payload, + const void *name, + void *opaque) +{ + udevSeclabelPtr list = payload; + const char *device = name; + virJSONValuePtr seclabels = opaque; + virJSONValuePtr deviceLabels = NULL; + virJSONValuePtr deviceJSON = NULL; + size_t i; + int ret = -1; + + if (!(deviceLabels = virJSONValueNewArray())) + return ret; + + for (i = 0; i < list->nseclabels; i++) { + virJSONValuePtr seclabel = udevSeclabelDump(list->seclabels[i]); + + if (!seclabel || + virJSONValueArrayAppend(deviceLabels, seclabel) < 0) { + virJSONValueFree(seclabel); + goto cleanup; + } + } + + if (!(deviceJSON = virJSONValueNewObject()) || + virJSONValueObjectAppendString(deviceJSON, "device", device) < 0 || + virJSONValueObjectAppend(deviceJSON, "labels", deviceLabels) < 0) + goto cleanup; + deviceLabels = NULL; + + if (virJSONValueArrayAppend(seclabels, deviceJSON) < 0) + goto cleanup; + deviceJSON = NULL; + + ret = 0; + cleanup: + virJSONValueFree(deviceJSON); + virJSONValueFree(deviceLabels); + return ret; +} + + static void virUdevMgrDispose(void *obj) { @@ -202,3 +266,96 @@ virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, virObjectUnlock(mgr); return ret; } + + +static virJSONValuePtr +virUdevSeclabelDump(virUdevMgrPtr mgr) +{ + virJSONValuePtr seclabels; + + if (!(seclabels = virJSONValueNewArray())) + return NULL; + + if (virHashForEach(mgr->labels, udevSeclabelsDump, seclabels) < 0) { + virJSONValueFree(seclabels); + return NULL; + } + + return seclabels; +} + + +static char * +virUdevMgrDumpInternal(virUdevMgrPtr mgr) +{ + virJSONValuePtr object = NULL; + virJSONValuePtr child = NULL; + char *ret = NULL; + + if (!(object = virJSONValueNewObject())) + goto cleanup; + + if (!(child = virUdevSeclabelDump(mgr))) + goto cleanup; + + if (virJSONValueObjectAppend(object, "labels", child) < 0) { + virJSONValueFree(child); + goto cleanup; + } + + ret = virJSONValueToString(object, true); + cleanup: + virJSONValueFree(object); + return ret; +} + + +char * +virUdevMgrDumpStr(virUdevMgrPtr mgr) +{ + char *ret; + + virObjectLock(mgr); + ret = virUdevMgrDumpInternal(mgr); + virObjectUnlock(mgr); + return ret; +} + + +static int +virUdevMgrRewriter(int fd, void *opaque) +{ + const char *str = opaque; + + return safewrite(fd, str, strlen(str)); +} + + +int +virUdevMgrDumpFile(virUdevMgrPtr mgr, + const char *filename) +{ + int ret = -1; + char *state; + + virObjectLock(mgr); + + if (!(state = virUdevMgrDumpInternal(mgr))) + goto cleanup; + + /* Here we shouldn't use pure virFileWriteStr() as that one is not atomic. + * We can be interrupted in the middle (e.g. due to a context switch) and + * thus leave the file partially written. */ + if (virFileRewrite(filename, 0644, virUdevMgrRewriter, state) < 0) { + virReportSystemError(errno, + _("Unable to save state file %s"), + filename); + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnlock(mgr); + VIR_FREE(state); + return ret; +} diff --git a/src/util/virudev.h b/src/util/virudev.h index 692b39b..1d5a23e 100644 --- a/src/util/virudev.h +++ b/src/util/virudev.h @@ -36,4 +36,9 @@ int virUdevMgrAddLabel(virUdevMgrPtr mgr, int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, const char *device); +char *virUdevMgrDumpStr(virUdevMgrPtr mgr); + +int virUdevMgrDumpFile(virUdevMgrPtr mgr, + const char *filename); + #endif -- 2.8.4

Now that we have the virudev module somewhat working, lets introduce some testing of it. We also need mock for virRandomBits function. Without it, the order in which entries occur in the hash table would be random and thus test would fail some times (as we expect certain ordering). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/Makefile.am | 12 +++ tests/virudevmock.c | 29 +++++++ tests/virudevtest.c | 124 ++++++++++++++++++++++++++++++ tests/virudevtestdata/complex.json | 30 ++++++++ tests/virudevtestdata/empty.json | 5 ++ tests/virudevtestdata/simple-dac.json | 13 ++++ tests/virudevtestdata/simple-selinux.json | 13 ++++ 7 files changed, 226 insertions(+) create mode 100644 tests/virudevmock.c create mode 100644 tests/virudevtest.c create mode 100644 tests/virudevtestdata/complex.json create mode 100644 tests/virudevtestdata/empty.json create mode 100644 tests/virudevtestdata/simple-dac.json create mode 100644 tests/virudevtestdata/simple-selinux.json diff --git a/tests/Makefile.am b/tests/Makefile.am index 924029a..824796b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -150,6 +150,7 @@ EXTRA_DIST = \ virpcitestdata \ virscsidata \ virsh-uriprecedence \ + virudevtestdata \ virusbtestdata \ vmwareverdata \ vmx2xmldata \ @@ -193,6 +194,7 @@ test_programs = virshtest sockettest \ vircaps2xmltest \ virnetdevtest \ virtypedparamtest \ + virudevtest \ $(NULL) if WITH_REMOTE @@ -406,6 +408,7 @@ test_libraries = libshunload.la \ virhostcpumock.la \ nssmock.la \ domaincapsmock.la \ + virudevmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -1343,6 +1346,15 @@ virtypedparamtest_SOURCES = \ virtypedparamtest.c testutils.h testutils.c virtypedparamtest_LDADD = $(LDADDS) +virudevtest_SOURCES = \ + virudevtest.c testutils.h testutils.c +virudevtest_LDADD = $(LDADDS) + +virudevmock_la_SOURCES = virudevmock.c +virudevmock_la_CFLAGS = $(AM_CFLAGS) +virudevmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virudevmock_la_LIBADD = $(MOCKLIBS_LIBS) + if WITH_LINUX fchosttest_SOURCES = \ diff --git a/tests/virudevmock.c b/tests/virudevmock.c new file mode 100644 index 0000000..d01f1c9 --- /dev/null +++ b/tests/virudevmock.c @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "virrandom.h" + +uint64_t virRandomBits(int nbits ATTRIBUTE_UNUSED) +{ + return 4; /* chosen by fair dice roll. + guaranteed to be random. */ +} diff --git a/tests/virudevtest.c b/tests/virudevtest.c new file mode 100644 index 0000000..883d751 --- /dev/null +++ b/tests/virudevtest.c @@ -0,0 +1,124 @@ +/* + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "testutils.h" +#include "virudev.h" +#include "virjson.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testUdevData { + const char *file; + const char *const *labels; +}; + + +static int +testDump(const void *opaque) +{ + const struct testUdevData *data = opaque; + virUdevMgrPtr mgr = NULL; + int ret = -1; + const char * const *tmp; + char *state = NULL; + char *filename = NULL; + + if (virAsprintf(&filename, "%s/virudevtestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (!(mgr = virUdevMgrNew())) + goto cleanup; + + tmp = data->labels; + while (*tmp) { + const char *device; + const char *model; + const char *label; + virSecurityDeviceLabelDefPtr seclabel; + + device = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + model = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + label = *tmp; + tmp++; + + if (!(seclabel = virSecurityDeviceLabelDefNewLabel(model, label))) + goto cleanup; + + if (virUdevMgrAddLabel(mgr, device, seclabel) < 0) + goto cleanup; + virSecurityDeviceLabelDefFree(seclabel); + } + + if (!(state = virUdevMgrDumpStr(mgr))) + goto cleanup; + + if (virTestCompareToFile(state, filename) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(filename); + VIR_FREE(state); + virObjectUnref(mgr); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST_DUMP(filename, ...) \ + do { \ + const char *labels[] = {__VA_ARGS__, NULL}; \ + struct testUdevData data = { \ + .file = filename, .labels = labels, \ + }; \ + if (virTestRun("Dump " filename, testDump, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_DUMP("empty", NULL); + DO_TEST_DUMP("simple-selinux", + "/dev/sda", "selinux", "someSELinuxLabel"); + DO_TEST_DUMP("simple-dac", + "/dev/sda", "dac", "someDACLabel"); + DO_TEST_DUMP("complex", + "/dev/sda", "dac", "someDACLabel", + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virudevmock.so") diff --git a/tests/virudevtestdata/complex.json b/tests/virudevtestdata/complex.json new file mode 100644 index 0000000..e1c8e69 --- /dev/null +++ b/tests/virudevtestdata/complex.json @@ -0,0 +1,30 @@ +{ + "labels": [ + { + "device": "/dev/sda", + "labels": [ + { + "model": "dac", + "label": "someDACLabel" + }, + { + "model": "selinux", + "label": "someSELinuxLabel" + } + ] + }, + { + "device": "/dev/sdb", + "labels": [ + { + "model": "dac", + "label": "otherDACLabel" + }, + { + "model": "selinux", + "label": "otherSELinuxLabel" + } + ] + } + ] +} diff --git a/tests/virudevtestdata/empty.json b/tests/virudevtestdata/empty.json new file mode 100644 index 0000000..763c0f2 --- /dev/null +++ b/tests/virudevtestdata/empty.json @@ -0,0 +1,5 @@ +{ + "labels": [ + + ] +} diff --git a/tests/virudevtestdata/simple-dac.json b/tests/virudevtestdata/simple-dac.json new file mode 100644 index 0000000..bbda6bb --- /dev/null +++ b/tests/virudevtestdata/simple-dac.json @@ -0,0 +1,13 @@ +{ + "labels": [ + { + "device": "/dev/sda", + "labels": [ + { + "model": "dac", + "label": "someDACLabel" + } + ] + } + ] +} diff --git a/tests/virudevtestdata/simple-selinux.json b/tests/virudevtestdata/simple-selinux.json new file mode 100644 index 0000000..7398d7f --- /dev/null +++ b/tests/virudevtestdata/simple-selinux.json @@ -0,0 +1,13 @@ +{ + "labels": [ + { + "device": "/dev/sda", + "labels": [ + { + "model": "selinux", + "label": "someSELinuxLabel" + } + ] + } + ] +} -- 2.8.4

This is the cherry on the top. For given device all its security labels are fetched. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virudev.c | 43 +++++++++++++++++++++++ src/util/virudev.h | 4 +++ tests/virudevtest.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0f08018..037443b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2581,6 +2581,7 @@ virTypedParamsValidate; virUdevMgrAddLabel; virUdevMgrDumpFile; virUdevMgrDumpStr; +virUdevMgrLookupLabels; virUdevMgrNew; virUdevMgrNewFromFile; virUdevMgrNewFromStr; diff --git a/src/util/virudev.c b/src/util/virudev.c index d60fbf9..9940f5f 100644 --- a/src/util/virudev.c +++ b/src/util/virudev.c @@ -300,6 +300,49 @@ virUdevMgrAddLabel(virUdevMgrPtr mgr, int +virUdevMgrLookupLabels(virUdevMgrPtr mgr, + const char *device, + virSecurityDeviceLabelDefPtr **seclabels, + size_t *nseclabels) +{ + int ret = -1; + udevSeclabelPtr list; + size_t i; + + virObjectLock(mgr); + + if (!(list = virHashLookup(mgr->labels, device))) { + *seclabels = NULL; + *nseclabels = 0; + ret = 0; + goto cleanup; + } + + if (VIR_ALLOC_N(*seclabels, list->nseclabels) < 0) + goto cleanup; + + *nseclabels = list->nseclabels; + + for (i = 0; i < list->nseclabels; i++) { + if (!((*seclabels)[i] = virSecurityDeviceLabelDefCopy(list->seclabels[i]))) + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnlock(mgr); + if (ret < 0) { + if (*seclabels) { + for (i = 0; i < *nseclabels; i++) + virSecurityDeviceLabelDefFree((*seclabels)[i]); + VIR_FREE(*seclabels); + } + } + return ret; +} + + +int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, const char *device) { diff --git a/src/util/virudev.h b/src/util/virudev.h index 82b2b4f..4e286bb 100644 --- a/src/util/virudev.h +++ b/src/util/virudev.h @@ -37,6 +37,10 @@ int virUdevMgrAddLabel(virUdevMgrPtr mgr, const virSecurityDeviceLabelDef *seclabel); int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, const char *device); +int virUdevMgrLookupLabels(virUdevMgrPtr mgr, + const char *device, + virSecurityDeviceLabelDefPtr **seclabels, + size_t *nseclabels); char *virUdevMgrDumpStr(virUdevMgrPtr mgr); diff --git a/tests/virudevtest.c b/tests/virudevtest.c index c395741..36a9077 100644 --- a/tests/virudevtest.c +++ b/tests/virudevtest.c @@ -124,6 +124,76 @@ testParse(const void *opaque) static int +testLookup(const void *opaque) +{ + const struct testUdevData *data = opaque; + virUdevMgrPtr mgr = NULL; + int ret = -1; + const char * const *tmp; + char *filename = NULL; + virSecurityDeviceLabelDefPtr *seclabels = NULL; + size_t i, nseclabels = 0; + + if (virAsprintf(&filename, "%s/virudevtestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (!(mgr = virUdevMgrNewFromFile(filename))) + goto cleanup; + + tmp = data->labels; + while (*tmp) { + const char *device; + const char *model; + const char *label; + + device = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + model = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + label = *tmp; + tmp++; + + if (virUdevMgrLookupLabels(mgr, device, &seclabels, &nseclabels) < 0) + goto cleanup; + + for (i = 0; i < nseclabels; i++) { + virSecurityDeviceLabelDefPtr seclabel = seclabels[i]; + + if (STREQ(seclabel->model, model) && + STREQ(seclabel->label, label)) + break; + } + + if (i == nseclabels) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Label not found"); + goto cleanup; + } + + for (i = 0; i < nseclabels; i++) + virSecurityDeviceLabelDefFree(seclabels[i]); + VIR_FREE(seclabels); + nseclabels = 0; + } + + ret = 0; + cleanup: + for (i = 0; i < nseclabels; i++) + virSecurityDeviceLabelDefFree(seclabels[i]); + VIR_FREE(seclabels); + VIR_FREE(filename); + virObjectUnref(mgr); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -147,6 +217,16 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_LOOKUP(filename, ...) \ + do { \ + const char *labels[] = {__VA_ARGS__, NULL}; \ + struct testUdevData data = { \ + .file = filename, .labels = labels, \ + }; \ + if (virTestRun("Lookup " filename, testLookup, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST_DUMP("empty", NULL); DO_TEST_DUMP("simple-selinux", "/dev/sda", "selinux", "someSELinuxLabel"); @@ -163,6 +243,17 @@ mymain(void) DO_TEST_PARSE("simple-dac"); DO_TEST_PARSE("complex"); + DO_TEST_LOOKUP("empty", NULL); + DO_TEST_LOOKUP("simple-selinux", + "/dev/sda", "selinux", "someSELinuxLabel"); + DO_TEST_LOOKUP("simple-dac", + "/dev/sda", "dac", "someDACLabel"); + DO_TEST_LOOKUP("complex", + "/dev/sda", "dac", "someDACLabel", + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.8.4

On Thu, Nov 03, 2016 at 08:19:02PM +0800, Michal Privoznik wrote: NB, this code path is never going to be reached for any disks that are marked shared or readonly, since we've still not got a solution for ref counting of shared disks. This will cause the JSON file database to grow without bound until the host is rebooted, which is undesirable and could cause performance issues for hosts which start/stop alot of guests with shared/readonly disks. Fixing the shared disks problem will require us to maintain a database of information about labelling/owership of files and devices, synchronized across hosts. IMHO it is undesirable for us to end of maintaining the same information in two separate places, once for udev and once for the more general case. If we address the more general case first, then the majority of this series is not required, as we would already have a suitable database of information. We would only need the udev specific helper to query it. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|

Now that security drivers are capable of writing into virUdevMgr module, we also need it to flush its internal database right after that. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 12 +++++++++++- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 9 +++++++-- src/qemu/qemu_hotplug.c | 35 ++++++++++++++++++++++++++++------- src/qemu/qemu_process.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.h | 3 +++ 6 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8cba755..f5be7c7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -641,13 +641,15 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) /* qemuDomainMasterKeyRemove: + * @driver: qemu driver data * @priv: Pointer to the domain private object * * Remove the traces of the master key, clear the heap, clear the file, * delete the file. */ void -qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) +qemuDomainMasterKeyRemove(virQEMUDriverPtr driver, + qemuDomainObjPrivatePtr priv) { char *path = NULL; @@ -660,6 +662,8 @@ qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) /* Delete the master key file */ path = qemuDomainGetMasterKeyFilePath(priv->libDir); unlink(path); + if (driver->udevMgr) + virUdevMgrRemoveAllLabels(driver->udevMgr, path); VIR_FREE(path); } @@ -4989,6 +4993,9 @@ qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver, vm->def, elem) < 0) VIR_WARN("Unable to restore security label on %s", NULLSTR(elem->path)); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + if (qemuTeardownImageCgroup(vm, elem) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(elem->path)); @@ -5028,6 +5035,9 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver, elem) < 0) goto cleanup; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2ee1829..a381b59 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -710,7 +710,8 @@ int qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver, int qemuDomainMasterKeyCreate(virDomainObjPtr vm); -void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv); +void qemuDomainMasterKeyRemove(virQEMUDriverPtr driver, + qemuDomainObjPrivatePtr priv); void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e791d40..d785c60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3868,8 +3868,11 @@ qemuDomainScreenshot(virDomainPtr dom, endjob: VIR_FORCE_CLOSE(tmp_fd); - if (unlink_tmp) + if (unlink_tmp) { + virSecurityManagerRestoreSavedStateLabel(driver->securityManager, vm->def, tmp); + qemuProcessFlushUdev(driver); unlink(tmp); + } VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); @@ -6693,6 +6696,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager, vm->def, path) < 0) VIR_WARN("failed to restore save state label on %s", path); + qemuProcessFlushUdev(driver); virObjectUnref(cfg); return ret; } @@ -16113,7 +16117,8 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, disk) < 0 || qemuSetupDiskCgroup(vm, disk) < 0 || virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, - disk) < 0)) + disk) < 0 || + qemuProcessFlushUdev(driver) < 0)) goto cleanup; disk->src = oldsrc; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e06862c..daadfbf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -113,6 +113,9 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, vm->def, disk) < 0) goto rollback_lock; + if (qemuProcessFlushUdev(driver) < 0) + goto rollback_label; + if (qemuSetupDiskCgroup(vm, disk) < 0) goto rollback_label; @@ -130,6 +133,9 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, VIR_WARN("Unable to restore security label on %s", virDomainDiskGetSource(disk)); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + rollback_lock: if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) VIR_WARN("Unable to release lock on %s", @@ -1427,6 +1433,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, goto error; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) teardownlabel = true; + if (qemuProcessFlushUdev(driver) < 0) + goto error; if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto error; @@ -1476,10 +1484,10 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0 || + qemuProcessFlushUdev(driver) < 0)) VIR_WARN("Unable to restore host device labelling on hotplug fail"); - if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); @@ -2253,6 +2261,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, vm->def, hostdev, NULL) < 0) goto cleanup; teardownlabel = true; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto cleanup; @@ -2280,8 +2290,9 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0 || + qemuProcessFlushUdev(driver) < 0)) VIR_WARN("Unable to restore host device labelling on hotplug fail"); if (added) qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); @@ -2355,6 +2366,9 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, goto cleanup; teardownlabel = true; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; + if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto cleanup; @@ -2398,8 +2412,9 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0 || + qemuProcessFlushUdev(driver) < 0)) VIR_WARN("Unable to restore host device labelling on hotplug fail"); } VIR_FREE(drivealias); @@ -3431,6 +3446,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + if (qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Failed to tear down cgroup for disk path %s", src); @@ -3609,6 +3627,9 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, vm->def, hostdev, NULL) < 0) VIR_WARN("Failed to restore host device labelling"); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Failed to remove host device cgroup ACL"); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b67aee..10f29ff 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5499,6 +5499,9 @@ qemuProcessLaunch(virConnectPtr conn, */ ret = -2; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; + if (incoming && incoming->fd != -1) { /* if there's an fd to migrate from, and it's a pipe, put the * proper security label on it @@ -5998,10 +6001,14 @@ void qemuProcessStop(virQEMUDriverPtr driver, } /* Remove the master key */ - qemuDomainMasterKeyRemove(priv); + qemuDomainMasterKeyRemove(driver, priv); virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); + if (driver->udevMgr) { + virUdevMgrRemoveAllLabels(driver->udevMgr, priv->libDir); + virUdevMgrRemoveAllLabels(driver->udevMgr, priv->channelTargetDir); + } qemuDomainClearPrivatePaths(vm); @@ -6033,10 +6040,12 @@ void qemuProcessStop(virQEMUDriverPtr driver, } /* Reset Security Labels unless caller don't want us to */ - if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) + if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) { virSecurityManagerRestoreAllLabel(driver->securityManager, vm->def, !!(flags & VIR_QEMU_PROCESS_STOP_MIGRATED)); + qemuProcessFlushUdev(driver); + } virSecurityManagerReleaseLabel(driver->securityManager, vm->def); for (i = 0; i < vm->def->ndisks; i++) { @@ -6587,3 +6596,37 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver, virHashFree(table); return ret; } + + +char * +qemuProcessGetUdevPath(virQEMUDriverPtr driver) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *file; + + ignore_value(virAsprintf(&file, "%s/devices.udev", cfg->stateDir)); + virObjectUnref(cfg); + return file; +} + + +int +qemuProcessFlushUdev(virQEMUDriverPtr driver) +{ + char *file = NULL; + int ret = -1; + + if (!driver->udevMgr) + return 0; + + if (!(file = qemuProcessGetUdevPath(driver))) + goto cleanup; + + if (virUdevMgrDumpFile(driver->udevMgr, file) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(file); + return ret; +} diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 21f3b0c..8d1e937 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -191,4 +191,7 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); +char * qemuProcessGetUdevPath(virQEMUDriverPtr driver); +int qemuProcessFlushUdev(virQEMUDriverPtr driver); + #endif /* __QEMU_PROCESS_H__ */ -- 2.8.4

If the daemon is restarted, the virUdevMgr loses its internal state. This is because entries to its internal DB are added whilst setting security labels. This obviously doesn't happen when the daemon is restarted. It's not wise to start with a fresh internal state and possibly leave behind previous one. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d785c60..7848a4e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -377,10 +377,20 @@ qemuSecurityInit(virQEMUDriverPtr driver) virSecurityManagerPtr stack = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; + char *file = NULL; - if (cfg->writeUdev && - !(driver->udevMgr = virUdevMgrNew())) - goto error; + if (cfg->writeUdev) { + if (!(file = qemuProcessGetUdevPath(driver))) + goto error; + + if (virFileExists(file)) { + if (!(driver->udevMgr = virUdevMgrNewFromFile(file))) + goto error; + } else { + if (!(driver->udevMgr = virUdevMgrNew())) + goto error; + } + } if (cfg->allowDiskFormatProbing) flags |= VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE; @@ -442,12 +452,14 @@ qemuSecurityInit(virQEMUDriverPtr driver) } driver->securityManager = stack; + VIR_FREE(file); virObjectUnref(cfg); return 0; error: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to initialize security drivers")); + VIR_FREE(file); virObjectUnref(stack); virObjectUnref(mgr); virObjectUnref(cfg); -- 2.8.4

In case of udev, it will never try to reset security label on say domain monitor socket, or some other channel. Therefore, it makes sense to filter those paths out and keep the state file on the disk small. The only paths that udev will handle are those prefixed with "/dev/". Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7848a4e..7773d73 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -370,6 +370,15 @@ qemuSecurityChownCallback(virStorageSourcePtr src, static int +qemuUdevFilter(const char *devpath, + const virSecurityDeviceLabelDef *seclabel ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + return STRPREFIX(devpath, "/dev/") ? 1 : 0; +} + + +static int qemuSecurityInit(virQEMUDriverPtr driver) { char **names; @@ -390,6 +399,8 @@ qemuSecurityInit(virQEMUDriverPtr driver) if (!(driver->udevMgr = virUdevMgrNew())) goto error; } + + virUdevMgrSetFilter(driver->udevMgr, qemuUdevFilter, NULL); } if (cfg->allowDiskFormatProbing) -- 2.8.4

In the previous patch we've introduced a configure check to see whether there's sufficiently new udev present in the system. I mean, the rule we've introduced relies on udev patches that were introduced in the 232 release. This one is not that widely present yet, therefore our spec file should count on that. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- libvirt.spec.in | 25 +++++++++++++++++++++++++ mingw-libvirt.spec.in | 2 -- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 856c702..d505df9 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -80,6 +80,7 @@ %define with_firewalld 0%{!?_without_firewalld:0} %define with_libssh2 0%{!?_without_libssh2:0} %define with_wireshark 0%{!?_without_wireshark:0} +%define with_udev_rules 0%{!?_without_udev_rules:0} %define with_pm_utils 1 # Finally set the OS / architecture specific special cases @@ -172,6 +173,11 @@ %define with_wireshark 0%{!?_without_wireshark:1} %endif +# XXX Currently feature we require from udev is not released yet +# pkg-config returns 0 on success, 1 on failure. Need to reverse the logic here. +%if !0%(pkg-config --atleast-version=232 libudev; echo $?) + %define with_udev_rules 1 +%endif %if %{with_qemu} || %{with_lxc} || %{with_uml} # numad is used to manage the CPU and memory placement dynamically, @@ -474,6 +480,9 @@ Requires: numad Requires: dbus # For uid creation during pre Requires(pre): shadow-utils +%if %{with_udev_rules} +Requires: systemd-udev >= 232 +%endif %description daemon Server side daemon required to manage the virtualization capabilities @@ -1100,6 +1109,12 @@ rm -rf .git %define arg_wireshark --without-wireshark-dissector %endif +%if %{with_udev_rules} + %define arg_udev_rules --with-udev-rules +%else + %define arg_udev_rules --without-udev-rules +%endif + %if %{with_pm_utils} %define arg_pm_utils --with-pm-utils %else @@ -1190,6 +1205,7 @@ rm -f po/stamp-po --with-driver-modules \ %{?arg_firewalld} \ %{?arg_wireshark} \ + %{?arg_udev_rules} \ %{?arg_pm_utils} \ --with-nss-plugin \ %{arg_packager} \ @@ -1230,6 +1246,12 @@ mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \ %endif %endif +%define _udevdir %(pkg-config --variable=udevdir udev) +%if ! %{with_udev_rules} +rm -f $RPM_BUILD_ROOT%{_libexecdir}/libvirt_udevhelper +rm -f $RPM_BUILD_ROOT%{_udevdir}/rules.d/99-libvirt.rules +%endif + install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}/lib/libvirt/dnsmasq/ # We don't want to install /etc/libvirt/qemu/networks in the main %files list # because if the admin wants to delete the default network completely, we don't @@ -1630,7 +1652,10 @@ exit 0 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/ %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper +%if %{with_udev_rules} %attr(0755, root, root) %{_libexecdir}/libvirt_udevhelper +%{_udevdir}/rules.d/99-libvirt.rules +%endif %attr(0755, root, root) %{_sbindir}/libvirtd %attr(0755, root, root) %{_sbindir}/virtlogd diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 015b06b..c9bf503 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -178,8 +178,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_datadir}/gtk-doc/* rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt_iohelper.exe rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt_iohelper.exe -rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt_udevhelper.exe -rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt_udevhelper.exe rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt-guests.sh rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh -- 2.8.4