4:36 a.m.
So far the whole transaction handling is done
virSecurityDACSetOwnershipInternal(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabelling it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/security/security_dac.c | 65 ++++++++++++++++++++++++++++++---------------
1 file changed, 44 insertions(+), 21 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 926c9a33c1..52e28b5fda 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -77,12 +77,13 @@ struct _virSecurityDACChownItem {
const virStorageSource *src;
uid_t uid;
gid_t gid;
+ bool restore;
};
typedef struct _virSecurityDACChownList virSecurityDACChownList;
typedef virSecurityDACChownList *virSecurityDACChownListPtr;
struct _virSecurityDACChownList {
- virSecurityDACDataPtr priv;
+ virSecurityManagerPtr manager;
virSecurityDACChownItemPtr *items;
size_t nItems;
};
@@ -95,7 +96,8 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
const char *path,
const virStorageSource *src,
uid_t uid,
- gid_t gid)
+ gid_t gid,
+ bool restore)
{
int ret = -1;
char *tmp = NULL;
@@ -111,6 +113,7 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
item->src = src;
item->uid = uid;
item->gid = gid;
+ item->restore = restore;
if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0)
goto cleanup;
@@ -159,25 +162,29 @@ static int
virSecurityDACTransactionAppend(const char *path,
const virStorageSource *src,
uid_t uid,
- gid_t gid)
+ gid_t gid,
+ bool restore)
{
virSecurityDACChownListPtr list = virThreadLocalGet(&chownList);
if (!list)
return 0;
- if (virSecurityDACChownListAppend(list, path, src, uid, gid) < 0)
+ if (virSecurityDACChownListAppend(list, path, src, uid, gid, restore) < 0)
return -1;
return 1;
}
-static int virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
- const virStorageSource *src,
- const char *path,
- uid_t uid,
- gid_t gid);
+static int virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
+ const virStorageSource *src,
+ const char *path,
+ uid_t uid,
+ gid_t gid);
+static int virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
+ const virStorageSource *src,
+ const char *path);
/**
* virSecurityDACTransactionRun:
* @pid: process pid
@@ -201,11 +208,16 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
virSecurityDACChownItemPtr item = list->items[i];
/* TODO Implement rollback */
- if (virSecurityDACSetOwnershipInternal(list->priv,
- item->src,
- item->path,
- item->uid,
- item->gid) < 0)
+ if ((!item->restore &&
+ virSecurityDACSetOwnership(list->manager,
+ item->src,
+ item->path,
+ item->uid,
+ item->gid) < 0) ||
+ (item->restore &&
+ virSecurityDACRestoreFileLabelInternal(list->manager,
+ item->src,
+ item->path) < 0))
return -1;
}
@@ -455,7 +467,6 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
static int
virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
{
- virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityDACChownListPtr list;
list = virThreadLocalGet(&chownList);
@@ -468,7 +479,7 @@ virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
if (VIR_ALLOC(list) < 0)
return -1;
- list->priv = priv;
+ list->manager = mgr;
if (virThreadLocalSet(&chownList, list) < 0) {
virReportSystemError(errno, "%s",
@@ -564,11 +575,6 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
/* Be aware that this function might run in a separate process.
* Therefore, any driver state changes would be thrown away. */
- if ((rc = virSecurityDACTransactionAppend(path, src, uid, gid)) < 0)
- return -1;
- else if (rc > 0)
- return 0;
-
VIR_INFO("Setting DAC user and group on '%s' to
'%ld:%ld'",
NULLSTR(src ? src->path : path), (long)uid, (long)gid);
@@ -640,11 +646,20 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
struct stat sb;
+ int rc;
if (!path && src && src->path &&
virStorageSourceIsLocalStorage(src))
path = src->path;
+ /* Be aware that this function might run in a separate process.
+ * Therefore, any driver state changes would be thrown away. */
+
+ if ((rc = virSecurityDACTransactionAppend(path, src, uid, gid, false)) < 0)
+ return -1;
+ else if (rc > 0)
+ return 0;
+
if (path) {
if (stat(path, &sb) < 0) {
virReportSystemError(errno, _("unable to stat: %s"), path);
@@ -676,6 +691,14 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
virStorageSourceIsLocalStorage(src))
path = src->path;
+ /* Be aware that this function might run in a separate process.
+ * Therefore, any driver state changes would be thrown away. */
+
+ if ((rv = virSecurityDACTransactionAppend(path, src, uid, gid, true)) < 0)
+ return -1;
+ else if (rv > 0)
+ return 0;
+
if (path) {
rv = virSecurityDACRecallLabel(priv, path, &uid, &gid);
if (rv < 0)
--
2.16.4