External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 05:35:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

virStorageSourceGetMetadata: Use depth limit instead of unique path checking

Browse Source 
Prevent unbounded chains by limiting the recursion depth of
virStorageSourceGetMetadataRecurse to the maximum number of image layers
we limit anyways.

This removes the last use of virStorageSourceGetUniqueIdentifier which
will allow us to delete some crusty old infrastructure which isn't
really needed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Peter Krempa 2021-03-22 17:21:12 +01:00
parent dc03aed6a1
commit a43c8763bf
5 changed files with 28 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/qemu/qemu_domain.c
View File

@ -7637,7 +7637,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
qemuDomainGetImageIds(cfg, vm, src, disksrc, &uid, &gid);
if (virStorageSourceGetMetadata(src, uid, gid, report_broken) < 0)
if (virStorageSourceGetMetadata(src, uid, gid,
QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH,
report_broken) < 0)
return -1;
for (n = src->backingStore; virStorageSourceIsBacking(n); n = n->backingStore) {

5
src/security/virt-aa-helper.c
View File

@ -937,9 +937,12 @@ get_files(vahControl * ctl)
continue;
/* XXX - if we knew the qemu user:group here we could send it in
* so that the open could be re-tried as that user:group.
*
* The maximum depth is limited to 200 layers similarly to the qemu
* implementation.
*/
if (!disk->src->backingStore)
virStorageSourceGetMetadata(disk->src, -1, -1, false);
virStorageSourceGetMetadata(disk->src, -1, -1, 200, false);
/* XXX should handle open errors more careful than just ignoring them.
*/

44
src/storage_file/storage_source.c
View File

@ -1297,11 +1297,9 @@ virStorageSourceGetMetadataRecurseReadHeader(virStorageSourcePtr src,
uid_t uid,
gid_t gid,
char **buf,
size_t *headerLen,
GHashTable *cycle)
size_t *headerLen)
{
int ret = -1;
const char *uniqueName;
ssize_t len;
if (virStorageSourceInitAs(src, uid, gid) < 0)
@ -1312,19 +1310,6 @@ virStorageSourceGetMetadataRecurseReadHeader(virStorageSourcePtr src,
goto cleanup;
}
if (!(uniqueName = virStorageSourceGetUniqueIdentifier(src)))
goto cleanup;
if (virHashHasEntry(cycle, uniqueName)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("backing store for %s (%s) is self-referential"),
NULLSTR(src->path), uniqueName);
goto cleanup;
}
if (virHashAddEntry(cycle, uniqueName, NULL) < 0)
goto cleanup;
if ((len = virStorageSourceRead(src, 0, VIR_STORAGE_MAX_HEADER, buf)) < 0)
goto cleanup;
@ -1343,7 +1328,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr src,
virStorageSourcePtr parent,
uid_t uid, gid_t gid,
bool report_broken,
GHashTable *cycle,
size_t max_depth,
unsigned int depth)
{
virStorageFileFormat orig_format = src->format;
@ -1352,9 +1337,16 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr src,
g_autofree char *buf = NULL;
g_autoptr(virStorageSource) backingStore = NULL;
VIR_DEBUG("path=%s format=%d uid=%u gid=%u",
VIR_DEBUG("path=%s format=%d uid=%u gid=%u depth=%u",
NULLSTR(src->path), src->format,
(unsigned int)uid, (unsigned int)gid);
(unsigned int)uid, (unsigned int)gid, depth);
if (depth > max_depth) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("backing store for %s is self-referential or too deeply nested"),
NULLSTR(src->path));
return -1;
}
if (src->format == VIR_STORAGE_FILE_AUTO_SAFE)
src->format = VIR_STORAGE_FILE_AUTO;
@ -1369,7 +1361,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr src,
}
if (virStorageSourceGetMetadataRecurseReadHeader(src, parent, uid, gid,
&buf, &headerLen, cycle) < 0)
&buf, &headerLen) < 0)
return -1;
if (virStorageFileProbeGetMetadata(src, buf, headerLen) < 0)
@ -1396,7 +1388,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr src,
if ((rv = virStorageSourceGetMetadataRecurse(backingStore, parent,
uid, gid,
report_broken,
cycle, depth + 1)) < 0) {
max_depth, depth + 1)) < 0) {
if (!report_broken)
return 0;
@ -1427,7 +1419,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr src,
* Extract metadata about the storage volume with the specified
* image format. If image format is VIR_STORAGE_FILE_AUTO, it
* will probe to automatically identify the format. Recurses through
* the entire chain.
* the chain up to @max_depth layers.
*
* Open files using UID and GID (or pass -1 for the current user/group).
* Treat any backing files without explicit type as raw, unless ALLOW_PROBE.
@ -1445,14 +1437,14 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr src,
int
virStorageSourceGetMetadata(virStorageSourcePtr src,
uid_t uid, gid_t gid,
size_t max_depth,
bool report_broken)
{
g_autoptr(GHashTable) cycle = virHashNew(NULL);
virStorageType actualType = virStorageSourceGetActualType(src);
VIR_DEBUG("path=%s format=%d uid=%u gid=%u report_broken=%d",
VIR_DEBUG("path=%s format=%d uid=%u gid=%u max_depth=%zu report_broken=%d",
src->path, src->format, (unsigned int)uid, (unsigned int)gid,
report_broken);
max_depth, report_broken);
if (src->format <= VIR_STORAGE_FILE_NONE) {
if (actualType == VIR_STORAGE_TYPE_DIR)
@ -1462,5 +1454,5 @@ virStorageSourceGetMetadata(virStorageSourcePtr src,
}
return virStorageSourceGetMetadataRecurse(src, src, uid, gid,
report_broken, cycle, 1);
report_broken, max_depth, 1);
}

1
src/storage_file/storage_source.h
View File

@ -134,6 +134,7 @@ virStorageSourceSupportsBackingChainTraversal(const virStorageSource *src);
int
virStorageSourceGetMetadata(virStorageSourcePtr src,
uid_t uid, gid_t gid,
size_t max_depth,
bool report_broken)
ATTRIBUTE_NONNULL(1);

3
tests/virstoragetest.c
View File

@ -101,7 +101,8 @@ testStorageFileGetMetadata(const char *path,
def->path = g_strdup(path);
if (virStorageSourceGetMetadata(def, uid, gid, true) < 0)
/* 20 is picked as an arbitrary depth, since the chains used here don't exceed it */
if (virStorageSourceGetMetadata(def, uid, gid, 20, true) < 0)
return NULL;
return g_steal_pointer(&def);