|
 |
a41c76 |
From 157a222a2001e3d5df4851ea3f43a00873f82be1 Mon Sep 17 00:00:00 2001
|
|
|
a41c76 |
Message-Id: <157a222a2001e3d5df4851ea3f43a00873f82be1@dist-git>
|
|
|
a41c76 |
From: Michal Privoznik <mprivozn@redhat.com>
|
|
|
a41c76 |
Date: Tue, 25 Feb 2020 11:24:50 +0100
|
|
|
a41c76 |
Subject: [PATCH] virSecurityManagerMetadataLock: Store locked paths
|
|
|
a41c76 |
|
|
|
a41c76 |
So far, in the lock state we are storing only the file
|
|
|
a41c76 |
descriptors of the files we've locked. Therefore, when unlocking
|
|
|
a41c76 |
them and something does wrong the only thing we can report is FD
|
|
|
a41c76 |
number, which is not user friendly at all. But if we store paths
|
|
|
a41c76 |
among with FDs we can do better error reporting.
|
|
|
a41c76 |
|
|
|
a41c76 |
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
|
|
a41c76 |
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
|
|
|
a41c76 |
(cherry picked from commit 256e01e59e922ff70dce56284e53e3463d4dc072)
|
|
|
a41c76 |
|
|
|
a41c76 |
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1804672
|
|
|
a41c76 |
|
|
|
a41c76 |
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
|
|
a41c76 |
Message-Id: <8b12c9adde3c8553b4a2b588ac1a657d0638336b.1582626185.git.mprivozn@redhat.com>
|
|
|
a41c76 |
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
|
|
|
a41c76 |
---
|
|
|
a41c76 |
src/security/security_manager.c | 21 +++++++++++++++------
|
|
|
a41c76 |
1 file changed, 15 insertions(+), 6 deletions(-)
|
|
|
a41c76 |
|
|
|
a41c76 |
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
|
|
|
a41c76 |
index f229d94570..05d20e36af 100644
|
|
|
a41c76 |
--- a/src/security/security_manager.c
|
|
|
a41c76 |
+++ b/src/security/security_manager.c
|
|
|
a41c76 |
@@ -1246,8 +1246,9 @@ virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
|
|
|
a41c76 |
|
|
|
a41c76 |
|
|
|
a41c76 |
struct _virSecurityManagerMetadataLockState {
|
|
|
a41c76 |
- size_t nfds;
|
|
|
a41c76 |
+ size_t nfds; /* Captures size of both @fds and @paths */
|
|
|
a41c76 |
int *fds;
|
|
|
a41c76 |
+ const char **paths;
|
|
|
a41c76 |
};
|
|
|
a41c76 |
|
|
|
a41c76 |
|
|
|
a41c76 |
@@ -1278,6 +1279,7 @@ cmpstringp(const void *p1, const void *p2)
|
|
|
a41c76 |
*
|
|
|
a41c76 |
* Lock passed @paths for metadata change. The returned state
|
|
|
a41c76 |
* should be passed to virSecurityManagerMetadataUnlock.
|
|
|
a41c76 |
+ * Passed @paths must not be freed until the corresponding unlock call.
|
|
|
a41c76 |
*
|
|
|
a41c76 |
* NOTE: this function is not thread safe (because of usage of
|
|
|
a41c76 |
* POSIX locks).
|
|
|
a41c76 |
@@ -1293,9 +1295,11 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
|
|
|
a41c76 |
size_t i = 0;
|
|
|
a41c76 |
size_t nfds = 0;
|
|
|
a41c76 |
int *fds = NULL;
|
|
|
a41c76 |
+ const char **locked_paths = NULL;
|
|
|
a41c76 |
virSecurityManagerMetadataLockStatePtr ret = NULL;
|
|
|
a41c76 |
|
|
|
a41c76 |
- if (VIR_ALLOC_N(fds, npaths) < 0)
|
|
|
a41c76 |
+ if (VIR_ALLOC_N(fds, npaths) < 0 ||
|
|
|
a41c76 |
+ VIR_ALLOC_N(locked_paths, npaths) < 0)
|
|
|
a41c76 |
return NULL;
|
|
|
a41c76 |
|
|
|
a41c76 |
/* Sort paths to lock in order to avoid deadlocks with other
|
|
|
a41c76 |
@@ -1372,12 +1376,14 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
|
|
|
a41c76 |
break;
|
|
|
a41c76 |
} while (1);
|
|
|
a41c76 |
|
|
|
a41c76 |
+ locked_paths[nfds] = p;
|
|
|
a41c76 |
VIR_APPEND_ELEMENT_COPY_INPLACE(fds, nfds, fd);
|
|
|
a41c76 |
}
|
|
|
a41c76 |
|
|
|
a41c76 |
if (VIR_ALLOC(ret) < 0)
|
|
|
a41c76 |
goto cleanup;
|
|
|
a41c76 |
|
|
|
a41c76 |
+ ret->paths = g_steal_pointer(&locked_paths);
|
|
|
a41c76 |
ret->fds = g_steal_pointer(&fds);
|
|
|
a41c76 |
ret->nfds = nfds;
|
|
|
a41c76 |
nfds = 0;
|
|
|
a41c76 |
@@ -1386,6 +1392,7 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
|
|
|
a41c76 |
for (i = nfds; i > 0; i--)
|
|
|
a41c76 |
VIR_FORCE_CLOSE(fds[i - 1]);
|
|
|
a41c76 |
VIR_FREE(fds);
|
|
|
a41c76 |
+ VIR_FREE(locked_paths);
|
|
|
a41c76 |
return ret;
|
|
|
a41c76 |
}
|
|
|
a41c76 |
|
|
|
a41c76 |
@@ -1401,21 +1408,23 @@ virSecurityManagerMetadataUnlock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
|
|
|
a41c76 |
|
|
|
a41c76 |
for (i = 0; i < (*state)->nfds; i++) {
|
|
|
a41c76 |
char ebuf[1024];
|
|
|
a41c76 |
+ const char *path = (*state)->paths[i];
|
|
|
a41c76 |
int fd = (*state)->fds[i];
|
|
|
a41c76 |
|
|
|
a41c76 |
/* Technically, unlock is not needed because it will
|
|
|
a41c76 |
* happen on VIR_CLOSE() anyway. But let's play it nice. */
|
|
|
a41c76 |
if (virFileUnlock(fd, METADATA_OFFSET, METADATA_LEN) < 0) {
|
|
|
a41c76 |
- VIR_WARN("Unable to unlock fd %d: %s",
|
|
|
a41c76 |
- fd, virStrerror(errno, ebuf, sizeof(ebuf)));
|
|
|
a41c76 |
+ VIR_WARN("Unable to unlock fd %d path %s: %s",
|
|
|
a41c76 |
+ fd, path, virStrerror(errno, ebuf, sizeof(ebuf)));
|
|
|
a41c76 |
}
|
|
|
a41c76 |
|
|
|
a41c76 |
if (VIR_CLOSE(fd) < 0) {
|
|
|
a41c76 |
- VIR_WARN("Unable to close fd %d: %s",
|
|
|
a41c76 |
- fd, virStrerror(errno, ebuf, sizeof(ebuf)));
|
|
|
a41c76 |
+ VIR_WARN("Unable to close fd %d path %s: %s",
|
|
|
a41c76 |
+ fd, path, virStrerror(errno, ebuf, sizeof(ebuf)));
|
|
|
a41c76 |
}
|
|
|
a41c76 |
}
|
|
|
a41c76 |
|
|
|
a41c76 |
VIR_FREE((*state)->fds);
|
|
|
a41c76 |
+ VIR_FREE((*state)->paths);
|
|
|
a41c76 |
VIR_FREE(*state);
|
|
|
a41c76 |
}
|
|
|
a41c76 |
--
|
|
|
a41c76 |
2.25.1
|
|
|
a41c76 |
|