fix various issues

[libvirt PATCH 02/11] domain_conf: fix NULL dereference on error in virDomainObjCopyPersistentDef

Posted by Pavel Hrdina 5 years, 2 months ago

The issue was introduced together with the function itself by commit
<da1eba6bc8f58bfce34136710d1979a3a44adb17>.  Calling
`virDomainObjGetPersistentDef` may return NULL which is later passed
to `virDomainDefFormat` where the `def` attribute is marked as NONNULL
and later in `virDomainDefFormatInternalSetRootName` it is actually
defererenced without any other check.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/domain_conf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5c30227212..eaad72ad0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30933,6 +30933,12 @@ virDomainObjCopyPersistentDef(virDomainObjPtr dom,
     virDomainDefPtr cur;
 
     cur = virDomainObjGetPersistentDef(xmlopt, dom, parseOpaque);
+    if (!cur) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Get persistent config failed"));
+        return NULL;
+    }
+
     return virDomainDefCopy(cur, xmlopt, parseOpaque, false);
 }
 
-- 
2.26.2

Re: [libvirt PATCH 02/11] domain_conf: fix NULL dereference on error in virDomainObjCopyPersistentDef

Posted by Peter Krempa 5 years, 2 months ago

On Mon, Nov 16, 2020 at 16:38:49 +0100, Pavel Hrdina wrote:
> The issue was introduced together with the function itself by commit
> <da1eba6bc8f58bfce34136710d1979a3a44adb17>.  Calling
> `virDomainObjGetPersistentDef` may return NULL which is later passed
> to `virDomainDefFormat` where the `def` attribute is marked as NONNULL
> and later in `virDomainDefFormatInternalSetRootName` it is actually
> defererenced without any other check.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/conf/domain_conf.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5c30227212..eaad72ad0a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30933,6 +30933,12 @@ virDomainObjCopyPersistentDef(virDomainObjPtr dom,
>      virDomainDefPtr cur;
>  
>      cur = virDomainObjGetPersistentDef(xmlopt, dom, parseOpaque);
> +    if (!cur) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Get persistent config failed"));

"failed to get persistent definition object"

Reviewed-by: Peter Krempa <pkrempa@redhat.com>