Re: [PATCH] qemuSnapshotDeleteValidate: Fix crash when disk is not found in VM definition

[Martin Kletzander]

On Fri, Nov 29, 2024 at 10:56:45PM +0800, kaihuan wrote:

qemuDomainDiskByName() can return a NULL pointer on failure.
But this returned value in qemuSnapshotDeleteValidate is not checked.It will 
make libvirtd crash.

Hi, looking through old unreviewed patches I found this one.  Sorry for
the wait.  Can you explain whether you found out how to trigger that?
This looks like there is some other issue that causes a snapshot having
a disk that is not in the domain.  Does that happen when you want to
delete a snapshot after unplugging a disk?

Signed-off-by: kaihuan <jungleman...@gmail.com>
---
src/qemu/qemu_snapshot.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 18b2e478f6..bcbd913073 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -4242,8 +4242,19 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
            virDomainDiskDef *vmdisk = NULL;
            virDomainDiskDef *disk = NULL;

-            vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name);
-            disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name);

The function calls already report an error when the disk is not found,
so there is not need to rewrite that error in my opinion.

+            if (!(vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name))) {
+                virReportError(VIR_ERR_OPERATION_FAILED,
+                            _("disk '%1$s' referenced by snapshot '%2$s' not found 
in the current definition"),
+                            snapDisk->name, snap->def->name);
+                return -1;
+            }
+
+            if (!(disk = qemuDomainDiskByName(snapdef->parent.dom, 
snapDisk->name))) {
+                virReportError(VIR_ERR_OPERATION_FAILED,
+                            _("disk '%1$s' referenced by snapshot '%2$s' not found 
in the VM definition of the deleted snapshot"),
+                            snapDisk->name, snap->def->name);
+                return -1;
+            }

            if (!virStorageSourceIsSameLocation(vmdisk->src, disk->src)) {
                virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
--
2.33.1.windows.1

signature.asc
Description: PGP signature