The code at the very bottom of the DAC secdriver that calls chown() should be fine with read-only data. If something needs to be prepared it should have been done beforehand. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 25 ++++++++++++++++--------- src/security/security_dac.c | 4 ++-- src/security/security_manager.h | 2 +- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_gluster.c | 2 +- src/storage/storage_driver.c | 6 +++--- src/storage/storage_driver.h | 4 ++-- src/util/virstoragefile.c | 2 +- src/util/virstoragefile.h | 2 +- 10 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 675a4d0e7..6a45ccd9b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -329,10 +329,11 @@ qemuAutostartDomains(virQEMUDriverPtr driver) static int -qemuSecurityChownCallback(virStorageSourcePtr src, +qemuSecurityChownCallback(const virStorageSource *src, uid_t uid, gid_t gid) { + virStorageSourcePtr cpy = NULL; struct stat sb; int save_errno = 0; int ret = -1; @@ -355,22 +356,28 @@ qemuSecurityChownCallback(virStorageSourcePtr src, } return chown(src->path, uid, gid); - } + } else { + if (!(cpy = virStorageSourceCopy(src, false))) + goto cleanup; - /* storage file init reports errors, return -2 on failure */ - if (virStorageFileInit(src) < 0) - return -2; + /* src file init reports errors, return -2 on failure */ + if (virStorageFileInit(cpy) < 0) { + ret = -2; + goto cleanup; + } - if (virStorageFileChown(src, uid, gid) < 0) { - save_errno = errno; - goto cleanup; + if (virStorageFileChown(cpy, uid, gid) < 0) { + save_errno = errno; + goto cleanup; + } } ret = 0; cleanup: - virStorageFileDeinit(src); errno = save_errno; + virStorageFileDeinit(cpy); + virStorageSourceFree(cpy); return ret; } diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 649219e52..b6f75fe1d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -279,8 +279,8 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr) } static int -virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv, - virStorageSourcePtr src, +virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv, + const virStorageSource *src, const char *path, uid_t uid, gid_t gid) diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 0d22cb15f..80fceddc8 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -62,7 +62,7 @@ int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, * @src. The callback shall return 0 on success, -1 on error and errno set (no * libvirt error reported) OR -2 and a libvirt error reported. */ typedef int -(*virSecurityManagerDACChownCallback)(virStorageSourcePtr src, +(*virSecurityManagerDACChownCallback)(const virStorageSource *src, uid_t uid, gid_t gid); diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 28e1a6517..82fbbbf6d 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -285,7 +285,7 @@ typedef int int mode); typedef int -(*virStorageFileBackendChown)(virStorageSourcePtr src, +(*virStorageFileBackendChown)(const virStorageSource *src, uid_t uid, gid_t gid); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index de0e8d57d..9de0c17c0 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1600,7 +1600,7 @@ virStorageFileBackendFileAccess(virStorageSourcePtr src, static int -virStorageFileBackendFileChown(virStorageSourcePtr src, +virStorageFileBackendFileChown(const virStorageSource *src, uid_t uid, gid_t gid) { diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 8e867043e..ae0611543 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -809,7 +809,7 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) static int -virStorageFileBackendGlusterChown(virStorageSourcePtr src, +virStorageFileBackendGlusterChown(const virStorageSource *src, uid_t uid, gid_t gid) { diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 5fc706634..8f1d3f04c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2848,7 +2848,7 @@ int storageRegister(void) /* ----------- file handlers cooperating with storage driver --------------- */ static bool -virStorageFileIsInitialized(virStorageSourcePtr src) +virStorageFileIsInitialized(const virStorageSource *src) { return src && src->drv; } @@ -2888,7 +2888,7 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) * driver to perform labelling */ bool -virStorageFileSupportsSecurityDriver(virStorageSourcePtr src) +virStorageFileSupportsSecurityDriver(const virStorageSource *src) { int actualType; virStorageFileBackendPtr backend; @@ -3179,7 +3179,7 @@ virStorageFileAccess(virStorageSourcePtr src, * by libvirt storage backend. */ int -virStorageFileChown(virStorageSourcePtr src, +virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid) { diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index 3f2549da5..682c9ff82 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -44,9 +44,9 @@ ssize_t virStorageFileReadHeader(virStorageSourcePtr src, char **buf); const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); int virStorageFileAccess(virStorageSourcePtr src, int mode); -int virStorageFileChown(virStorageSourcePtr src, uid_t uid, gid_t gid); +int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid); -bool virStorageFileSupportsSecurityDriver(virStorageSourcePtr src); +bool virStorageFileSupportsSecurityDriver(const virStorageSource *src); int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ce6d21388..3d4bda700 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2082,7 +2082,7 @@ virStorageSourceGetActualType(const virStorageSource *def) bool -virStorageSourceIsLocalStorage(virStorageSourcePtr src) +virStorageSourceIsLocalStorage(const virStorageSource *src) { virStorageType type = virStorageSourceGetActualType(src); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1f62244db..bea1c35a2 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -349,7 +349,7 @@ int virStorageSourceInitChainElement(virStorageSourcePtr newelem, void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def); void virStorageSourceClear(virStorageSourcePtr def); int virStorageSourceGetActualType(const virStorageSource *def); -bool virStorageSourceIsLocalStorage(virStorageSourcePtr src); +bool virStorageSourceIsLocalStorage(const virStorageSource *src); bool virStorageSourceIsEmpty(virStorageSourcePtr src); bool virStorageSourceIsBlockLocal(const virStorageSource *src); void virStorageSourceFree(virStorageSourcePtr def); -- 2.11.0

With our new qemu namespace code in place, the relabelling of devices is done not as good is it could: a child process is spawned, it enters the mount namespace of the qemu process and then runs desired API of the security driver. Problem with this approach is that internal state transition of the security driver done in the child process is not reflected in the parent process. While currently it wouldn't matter that much, it is fairly easy to forget about that. We should take the extra step now while this limitation is still fresh in our minds. Three new APIs are introduced here: virSecurityManagerTransactionStart() virSecurityManagerTransactionCommit() virSecurityManagerTransactionAbort() The Start() is going to be used to let security driver know that we are starting a new transaction. During a transaction no security labels are actually touched, but rather recorded and only at Commit() phase they are actually updated. Should something go wrong Abort() aborts the transaction freeing up all memory allocated by transaction. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 3 ++ src/security/security_driver.h | 9 ++++++ src/security/security_manager.c | 68 +++++++++++++++++++++++++++++++++++++++++ src/security/security_manager.h | 5 +++ src/security/security_stack.c | 49 +++++++++++++++++++++++++++++ 5 files changed, 134 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c5b9cee45..97c1123b3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1176,6 +1176,9 @@ virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; virSecurityManagerStackAddNested; +virSecurityManagerTransactionAbort; +virSecurityManagerTransactionCommit; +virSecurityManagerTransactionStart; virSecurityManagerVerify; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index b48f2aaed..fa65eb359 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -51,6 +51,11 @@ typedef const char *(*virSecurityDriverGetBaseLabel) (virSecurityManagerPtr mgr, typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr); +typedef int (*virSecurityDriverTransactionStart) (virSecurityManagerPtr mgr); +typedef int (*virSecurityDriverTransactionCommit) (virSecurityManagerPtr mgr, + pid_t pid); +typedef void (*virSecurityDriverTransactionAbort) (virSecurityManagerPtr mgr); + typedef int (*virSecurityDomainRestoreDiskLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk); @@ -135,6 +140,10 @@ struct _virSecurityDriver { virSecurityDriverPreFork preFork; + virSecurityDriverTransactionStart transactionStart; + virSecurityDriverTransactionCommit transactionCommit; + virSecurityDriverTransactionAbort transactionAbort; + virSecurityDomainSecurityVerify domainSecurityVerify; virSecurityDomainSetDiskLabel domainSetSecurityDiskLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index f98c7d2d1..d8c6facc8 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -235,6 +235,74 @@ virSecurityManagerPostFork(virSecurityManagerPtr mgr) virObjectUnlock(mgr); } + +/** + * virSecurityManagerTransactionStart: + * @mgr: security manager + * + * Starts a new transaction. In transaction nothing is changed security + * label until virSecurityManagerTransactionCommit() is called. + * + * Returns 0 on success, + * -1 otherwise. + */ +int +virSecurityManagerTransactionStart(virSecurityManagerPtr mgr) +{ + int ret = 0; + + virObjectLock(mgr); + if (mgr->drv->transactionStart) + ret = mgr->drv->transactionStart(mgr); + virObjectUnlock(mgr); + return ret; +} + + +/** + * virSecurityManagerTransactionCommit: + * @mgr: security manager + * @pid: domain's PID + * + * Enters the @pid namespace (usually @pid refers to a domain) and + * performs all the operations on the transaction list. Note that the + * transaction is also freed, therefore new one has to be started after + * successful return from this function. Also it is considered as error + * if there's no transaction set and this function is called. + * + * Returns: 0 on success, + * -1 otherwise. + */ +int +virSecurityManagerTransactionCommit(virSecurityManagerPtr mgr, + pid_t pid) +{ + int ret = 0; + + virObjectLock(mgr); + if (mgr->drv->transactionCommit) + ret = mgr->drv->transactionCommit(mgr, pid); + virObjectUnlock(mgr); + return ret; +} + + +/** + * virSecurityManagerTransactionAbort: + * @mgr: security manager + * + * Cancels and frees any out standing transaction. + */ +void +virSecurityManagerTransactionAbort(virSecurityManagerPtr mgr) +{ + virObjectLock(mgr); + if (mgr->drv->transactionAbort) + mgr->drv->transactionAbort(mgr); + virObjectUnlock(mgr); +} + + void * virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr) { diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 80fceddc8..bae8493ee 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -76,6 +76,11 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, int virSecurityManagerPreFork(virSecurityManagerPtr mgr); void virSecurityManagerPostFork(virSecurityManagerPtr mgr); +int virSecurityManagerTransactionStart(virSecurityManagerPtr mgr); +int virSecurityManagerTransactionCommit(virSecurityManagerPtr mgr, + pid_t pid); +void virSecurityManagerTransactionAbort(virSecurityManagerPtr mgr); + void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDriver(virSecurityManagerPtr mgr); diff --git a/src/security/security_stack.c b/src/security/security_stack.c index c123931a0..6056ae321 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -137,6 +137,51 @@ virSecurityStackPreFork(virSecurityManagerPtr mgr) return rc; } + +static int +virSecurityStackTransactionStart(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerTransactionStart(item->securityManager) < 0) + rc = -1; + } + + return rc; +} + + +static int +virSecurityStackTransactionCommit(virSecurityManagerPtr mgr, + pid_t pid) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerTransactionCommit(item->securityManager, pid) < 0) + rc = -1; + } + + return rc; +} + + +static void +virSecurityStackTransactionAbort(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + + for (; item; item = item->next) + virSecurityManagerTransactionAbort(item->securityManager); +} + + static int virSecurityStackVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) @@ -612,6 +657,10 @@ virSecurityDriver virSecurityDriverStack = { .preFork = virSecurityStackPreFork, + .transactionStart = virSecurityStackTransactionStart, + .transactionCommit = virSecurityStackTransactionCommit, + .transactionAbort = virSecurityStackTransactionAbort, + .domainSecurityVerify = virSecurityStackVerify, .domainSetSecurityDiskLabel = virSecurityStackSetDiskLabel, -- 2.11.0

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/security/security_selinux.c | 256 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 253 insertions(+), 3 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index fec19b98c..92267e8fc 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -78,6 +78,22 @@ struct _virSecuritySELinuxCallbackData { virDomainDefPtr def; }; +typedef struct _virSecuritySELinuxContextItem virSecuritySELinuxContextItem; +typedef virSecuritySELinuxContextItem *virSecuritySELinuxContextItemPtr; +struct _virSecuritySELinuxContextItem { + const char *path; + const char *tcon; + bool optional; +}; + +typedef struct _virSecuritySELinuxContextList virSecuritySELinuxContextList; +typedef virSecuritySELinuxContextList *virSecuritySELinuxContextListPtr; +struct _virSecuritySELinuxContextList { + bool privileged; + virSecuritySELinuxContextItemPtr *items; + size_t nItems; +}; + #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux" @@ -87,6 +103,115 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr, virDomainTPMDefPtr tpm); +virThreadLocal contextList; + +static int +virSecuritySELinuxContextListAppend(virSecuritySELinuxContextListPtr list, + const char *path, + const char *tcon, + bool optional) +{ + virSecuritySELinuxContextItemPtr item; + + if (VIR_ALLOC(item) < 0) + return -1; + + item->path = path; + item->tcon = tcon; + item->optional = optional; + + if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0) { + VIR_FREE(item); + return -1; + } + + return 0; +} + +static void +virSecuritySELinuxContextListFree(void *opaque) +{ + virSecuritySELinuxContextListPtr list = opaque; + size_t i; + + if (!list) + return; + + for (i = 0; i < list->nItems; i++) + VIR_FREE(list->items[i]); + VIR_FREE(list); +} + + +/** + * virSecuritySELinuxTransactionAppend: + * @path: Path to chown + * @tcon: target context + * @optional: true if setting @tcon is optional + * + * Appends an entry onto transaction list. + * + * Returns: 1 in case of successful append + * 0 if there is no transaction enabled + * -1 otherwise. + */ +static int +virSecuritySELinuxTransactionAppend(const char *path, + const char *tcon, + bool optional) +{ + virSecuritySELinuxContextListPtr list; + + list = virThreadLocalGet(&contextList); + if (!list) + return 0; + + if (virSecuritySELinuxContextListAppend(list, path, tcon, optional) < 0) + return -1; + + return 1; +} + + +static int virSecuritySELinuxSetFileconHelper(const char *path, + const char *tcon, + bool optional, + bool privileged); + +/** + * virSecuritySELinuxTransactionRun: + * @pid: process pid + * @opaque: opaque data + * + * This is the callback that runs in the same namespace as the domain we are + * relabelling. For given transaction (@opaque) it relabels all the paths on + * the list. + * + * Returns: 0 on success + * -1 otherwise. + */ +static int +virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED, + void *opaque) +{ + virSecuritySELinuxContextListPtr list = opaque; + size_t i; + + for (i = 0; i < list->nItems; i++) { + virSecuritySELinuxContextItemPtr item = list->items[i]; + + /* TODO Implement rollback */ + if (virSecuritySELinuxSetFileconHelper(item->path, + item->tcon, + item->optional, + list->privileged) < 0) + return -1; + } + + return 0; +} + + /* * Returns 0 on success, 1 if already reserved, or -1 on fatal error */ @@ -560,6 +685,14 @@ static int virSecuritySELinuxInitialize(virSecurityManagerPtr mgr) { VIR_DEBUG("SELinuxInitialize %s", virSecurityManagerGetDriver(mgr)); + + if (virThreadLocalInit(&contextList, + virSecuritySELinuxContextListFree) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize thread local variable")); + return -1; + } + if (STREQ(virSecurityManagerGetDriver(mgr), "LXC")) { return virSecuritySELinuxLXCInitialize(mgr); } else { @@ -843,6 +976,110 @@ virSecuritySELinuxGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) return SECURITY_SELINUX_VOID_DOI; } +/** + * virSecuritySELinuxTransactionStart: + * @mgr: security manager + * + * Starts a new transaction. In transaction nothing is changed context + * until TransactionCommit() is called. This is implemented as a list + * that is appended to whenever setfilecon() would be called. Since + * secdriver APIs can be called from multiple threads (to work over + * different domains) the pointer to the list is stored in thread local + * variable. + * + * Returns 0 on success, + * -1 otherwise. + */ +static int +virSecuritySELinuxTransactionStart(virSecurityManagerPtr mgr) +{ + bool privileged = virSecurityManagerGetPrivileged(mgr); + virSecuritySELinuxContextListPtr list; + + list = virThreadLocalGet(&contextList); + if (list) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Another relabel transaction is already started")); + return -1; + } + + if (VIR_ALLOC(list) < 0) + return -1; + + list->privileged = privileged; + + if (virThreadLocalSet(&contextList, list) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local variable")); + VIR_FREE(list); + return -1; + } + + return 0; +} + +/** + * virSecuritySELinuxTransactionCommit: + * @mgr: security manager + * @pid: domain's PID + * + * Enters the @pid namespace (usually @pid refers to a domain) and + * performs all the sefilecon()-s on the list. Note that the + * transaction is also freed, therefore new one has to be started after + * successful return from this function. Also it is considered as error + * if there's no transaction set and this function is called. + * + * Returns: 0 on success, + * -1 otherwise. + */ +static int +virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + pid_t pid) +{ + virSecuritySELinuxContextListPtr list; + int ret = 0; + + list = virThreadLocalGet(&contextList); + if (!list) + return 0; + + if (virThreadLocalSet(&contextList, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Unable to clear thread local variable")); + goto cleanup; + } + + if (virProcessRunInMountNamespace(pid, + virSecuritySELinuxTransactionRun, + list) < 0) + goto cleanup; + + ret = 0; + cleanup: + virSecuritySELinuxContextListFree(list); + return ret; +} + +/** + * virSecuritySELinuxTransactionAbort: + * @mgr: security manager + * + * Cancels and frees any out standing transaction. + */ +static void +virSecuritySELinuxTransactionAbort(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + virSecuritySELinuxContextListPtr list; + + list = virThreadLocalGet(&contextList); + if (!list) + return; + + if (virThreadLocalSet(&contextList, NULL) < 0) + VIR_DEBUG("Unable to clear thread local variable"); + virSecuritySELinuxContextListFree(list); +} + static int virSecuritySELinuxGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def ATTRIBUTE_UNUSED, @@ -885,10 +1122,19 @@ virSecuritySELinuxGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, * return 1 if labelling was not possible. Otherwise, require a label * change, and return 0 for success, -1 for failure. */ static int -virSecuritySELinuxSetFileconHelper(const char *path, char *tcon, +virSecuritySELinuxSetFileconHelper(const char *path, const char *tcon, bool optional, bool privileged) { security_context_t econ; + int rc; + + /* Be aware that this function might run in a separate process. + * Therefore, any driver state changes would be thrown away. */ + + if ((rc = virSecuritySELinuxTransactionAppend(path, tcon, optional)) < 0) + return -1; + else if (rc > 0) + return 0; VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon); @@ -945,7 +1191,7 @@ virSecuritySELinuxSetFileconHelper(const char *path, char *tcon, static int virSecuritySELinuxSetFileconOptional(virSecurityManagerPtr mgr, - const char *path, char *tcon) + const char *path, const char *tcon) { bool privileged = virSecurityManagerGetPrivileged(mgr); return virSecuritySELinuxSetFileconHelper(path, tcon, true, privileged); @@ -953,7 +1199,7 @@ virSecuritySELinuxSetFileconOptional(virSecurityManagerPtr mgr, static int virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr, - const char *path, char *tcon) + const char *path, const char *tcon) { bool privileged = virSecurityManagerGetPrivileged(mgr); return virSecuritySELinuxSetFileconHelper(path, tcon, false, privileged); @@ -2667,6 +2913,10 @@ virSecurityDriver virSecurityDriverSELinux = { .getModel = virSecuritySELinuxGetModel, .getDOI = virSecuritySELinuxGetDOI, + .transactionStart = virSecuritySELinuxTransactionStart, + .transactionCommit = virSecuritySELinuxTransactionCommit, + .transactionAbort = virSecuritySELinuxTransactionAbort, + .domainSecurityVerify = virSecuritySELinuxVerify, .domainSetSecurityDiskLabel = virSecuritySELinuxSetDiskLabel, -- 2.11.0

On 01/09/2017 07:58 AM, Michal Privoznik wrote: s/is/to/ s/dameon/daemon Having trouble sleeping lately? ;-) Both paths have removed the PreFork/PostFork processing... Is that then no longer required? This is/was the only PreFork caller I think. Once we've entered Commit - calling Abort is a noop since Commit takes the 'list', clears the thread local storage, and will free the list on exit. This would seemingly be the only place where goto cleanup is necessary John