[PATCH] virshFindDisk: fix NULL-dereference of xmlCopyNode() result
xmlCopyNode() may return NULL. Add a check and log an error in this case. Found by Linux Verification Center (linuxtesting.org) with Svace. Fixes: 22766a1a53 ("virshFindDisk: Sanitize use of 'tmp' variable") Signed-off-by: Anastasia Belova <abelova@astralinux.ru> --- tools/virsh-domain.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 7253596f4a..8752294784 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12875,6 +12875,10 @@ virshFindDisk(const char *doc, STREQ_NULLABLE(sourceDir, path) || STREQ_NULLABLE(sourceName, path)) { xmlNodePtr ret = xmlCopyNode(nodes[i], 1); + if (!ret) { + vshError(NULL, "%s", _("Failed to copy node")); + return NULL; + } /* drop backing store since they are not needed here */ virshDiskDropBackingStore(ret); return ret; -- 2.43.0
On Wed, Jul 16, 2025 at 16:42:14 +0300, Anastasia Belova wrote:
xmlCopyNode() may return NULL. Add a check and log an error in this case.
Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes: 22766a1a53 ("virshFindDisk: Sanitize use of 'tmp' variable") Signed-off-by: Anastasia Belova <abelova@astralinux.ru> --- tools/virsh-domain.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 7253596f4a..8752294784 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12875,6 +12875,10 @@ virshFindDisk(const char *doc, STREQ_NULLABLE(sourceDir, path) || STREQ_NULLABLE(sourceName, path)) { xmlNodePtr ret = xmlCopyNode(nodes[i], 1); + if (!ret) { + vshError(NULL, "%s", _("Failed to copy node"));
I've changed this to "Failed to copy XML node" as that message is already used in our code and thus also translated.
+ return NULL; + } /* drop backing store since they are not needed here */ virshDiskDropBackingStore(ret); return ret; -- 2.43.0
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Thu, Jul 17, 2025 at 12:55:19PM +0200, Peter Krempa via Devel wrote:
On Wed, Jul 16, 2025 at 16:42:14 +0300, Anastasia Belova wrote:
xmlCopyNode() may return NULL. Add a check and log an error in this case.
Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes: 22766a1a53 ("virshFindDisk: Sanitize use of 'tmp' variable") Signed-off-by: Anastasia Belova <abelova@astralinux.ru> --- tools/virsh-domain.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 7253596f4a..8752294784 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12875,6 +12875,10 @@ virshFindDisk(const char *doc, STREQ_NULLABLE(sourceDir, path) || STREQ_NULLABLE(sourceName, path)) { xmlNodePtr ret = xmlCopyNode(nodes[i], 1); + if (!ret) { + vshError(NULL, "%s", _("Failed to copy node"));
I've changed this to "Failed to copy XML node" as that message is already used in our code and thus also translated.
FWIW, there are several other examples of xmlCopyNode return value not being checked across the code.
+ return NULL; + } /* drop backing store since they are not needed here */ virshDiskDropBackingStore(ret); return ret; -- 2.43.0
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Anastasia Belova -
Daniel P. Berrangé -
Peter Krempa