10:57 a.m.
Peter Krempa (2):
qemuBlockJobProcessEventCompletedPull: Avoid dangling pointer after
blockpull
qemuBlockJobProcessEventCompletedPull: Add backingStore terminators if
base is NULL
src/qemu/qemu_blockjob.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
--
2.30.2
10:57 a.m.
New subject: [PATCH 1/2] qemuBlockJobProcessEventCompletedPull: Avoid dangling pointer after blockpull
When doing a full block pull job (base == NULL) and the config XML
contains a compatible disk, the completer function would leave a
dangling pointer in 'cfgdisk->src->backingStore' as cfgdisk->src would
be set to the value of 'cfgbase' which was always set to
'cfgdisk->src->backingStore'.
This is wrong though since for the live definition XML we set the
respective counterpart to 'job->data.pull.base' which is NULL in the
above scenario.
This leads to a invalid pointer read when saving the config XML and may
end up in a crash.
Resolve it by setting 'cfgbase' only when 'job->data.pull.base' is
non-NULL.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1946918
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 66268a365a..d708fd18fd 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1005,10 +1005,7 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
if (!job->disk)
return;
- if ((cfgdisk = qemuBlockJobGetConfigDisk(vm, job->disk, job->data.pull.base)))
- cfgbase = cfgdisk->src->backingStore;
-
- if (!cfgdisk)
+ if (!(cfgdisk = qemuBlockJobGetConfigDisk(vm, job->disk,
job->data.pull.base)))
qemuBlockJobClearConfigChain(vm, job->disk);
qemuBlockJobProcessEventCompletedPullBitmaps(vm, job, asyncJob);
@@ -1018,6 +1015,8 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
return;
if (job->data.pull.base) {
+ if (cfgdisk)
+ cfgbase = cfgdisk->src->backingStore;
for (n = job->disk->src->backingStore; n && n !=
job->data.pull.base; n = n->backingStore) {
/* find the image on top of 'base' */
--
2.30.2
10:57 a.m.
New subject: [PATCH 2/2] qemuBlockJobProcessEventCompletedPull: Add backingStore terminators if base is NULL
When doing a blockpull with NULL base the full contents of the disk are
pulled into the topmost image which then becomes fullu self-contained.
qemuBlockJobProcessEventCompletedPull doesn't install the backing chain
terminators though, although it's guaranteed that there will be no
backing chain behind disk->src.
Add the terminators for completness and for disabling backing chain
detection on further boots.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index d708fd18fd..d2a769136d 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -992,6 +992,7 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
qemuBlockJobDataPtr job,
qemuDomainAsyncJob asyncJob)
{
+ virStorageSource *base = NULL;
virStorageSourcePtr baseparent = NULL;
virDomainDiskDefPtr cfgdisk = NULL;
virStorageSourcePtr cfgbase = NULL;
@@ -1015,8 +1016,11 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
return;
if (job->data.pull.base) {
+ base = job->data.pull.base;
+
if (cfgdisk)
cfgbase = cfgdisk->src->backingStore;
+
for (n = job->disk->src->backingStore; n && n !=
job->data.pull.base; n = n->backingStore) {
/* find the image on top of 'base' */
@@ -1027,10 +1031,17 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
baseparent = n;
}
+ } else {
+ /* create terminators for the chain; since we are pulling everything
+ * into the top image the chain is automatically considered terminated */
+ base = virStorageSourceNew();
+
+ if (cfgdisk)
+ cfgbase = virStorageSourceNew();
}
tmp = job->disk->src->backingStore;
- job->disk->src->backingStore = job->data.pull.base;
+ job->disk->src->backingStore = base;
if (baseparent)
baseparent->backingStore = NULL;
qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, tmp);
--
2.30.2
2:31 p.m.
New subject: [PATCH 2/2] qemuBlockJobProcessEventCompletedPull: Add backingStore terminators if base is NULL
On Mon, Apr 12, 2021 at 05:57:46PM +0200, Peter Krempa wrote:
When doing a blockpull with NULL base the full contents of the disk
are
pulled into the topmost image which then becomes fullu self-contained.
s/fullu/fully/
qemuBlockJobProcessEventCompletedPull doesn't install the backing
chain
terminators though, although it's guaranteed that there will be no
backing chain behind disk->src.
Add the terminators for completness and for disabling backing chain
detection on further boots.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index d708fd18fd..d2a769136d 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -992,6 +992,7 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
qemuBlockJobDataPtr job,
qemuDomainAsyncJob asyncJob)
{
+ virStorageSource *base = NULL;
virStorageSourcePtr baseparent = NULL;
virDomainDiskDefPtr cfgdisk = NULL;
virStorageSourcePtr cfgbase = NULL;
@@ -1015,8 +1016,11 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
return;
if (job->data.pull.base) {
+ base = job->data.pull.base;
+
if (cfgdisk)
cfgbase = cfgdisk->src->backingStore;
+
for (n = job->disk->src->backingStore; n && n !=
job->data.pull.base; n = n->backingStore) {
/* find the image on top of 'base' */
@@ -1027,10 +1031,17 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
baseparent = n;
}
+ } else {
+ /* create terminators for the chain; since we are pulling everything
+ * into the top image the chain is automatically considered terminated */
+ base = virStorageSourceNew();
+
+ if (cfgdisk)
+ cfgbase = virStorageSourceNew();
}
tmp = job->disk->src->backingStore;
- job->disk->src->backingStore = job->data.pull.base;
+ job->disk->src->backingStore = base;
if (baseparent)
baseparent->backingStore = NULL;
qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, tmp);
--
2.30.2
2:31 p.m.
On Mon, Apr 12, 2021 at 05:57:44PM +0200, Peter Krempa wrote:
Peter Krempa (2):
qemuBlockJobProcessEventCompletedPull: Avoid dangling pointer after
blockpull
qemuBlockJobProcessEventCompletedPull: Add backingStore terminators if
base is NULL
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
1323
days inactive
1323
days old
4 comments
2 participants
participants (2)
-
Pavel Hrdina -
Peter Krempa