[libvirt] [PATCH] virDomainMomentAssignDef: Don't dereference a NULL pointer

Michal Privoznik posted 1 patch 12 weeks ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/83bed621584d5cf293de4aad7d83b763ecd872f1.1553247764.git.mprivozn@redhat.com
src/conf/virdomainmomentobjlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

[libvirt] [PATCH] virDomainMomentAssignDef: Don't dereference a NULL pointer

Posted by Michal Privoznik 12 weeks ago
This functions tries to add a domain moment (love the name!) onto
a list of domain moments. Firstly, it checks if another moment
with the same name already exists. Then, it creates an empty
moment (without initializing its definition) and tries to add the
moment onto the list dereferencing moment definition in that
process. If it succeeds (which it never can), only after that it
sets moment->def.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/virdomainmomentobjlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
index b2122e7292..01e50ae1d4 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -227,7 +227,7 @@ virDomainMomentAssignDef(virDomainMomentObjListPtr moments,
     if (!(moment = virDomainMomentObjNew()))
         return NULL;
 
-    if (virHashAddEntry(moments->objs, moment->def->name, moment) < 0) {
+    if (virHashAddEntry(moments->objs, def->name, moment) < 0) {
         VIR_FREE(moment);
         return NULL;
     }
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virDomainMomentAssignDef: Don't dereference a NULL pointer

Posted by Eric Blake 12 weeks ago
On 3/22/19 4:42 AM, Michal Privoznik wrote:
> This functions tries to add a domain moment (love the name!) onto
> a list of domain moments. Firstly, it checks if another moment
> with the same name already exists. Then, it creates an empty
> moment (without initializing its definition) and tries to add the
> moment onto the list dereferencing moment definition in that
> process. If it succeeds (which it never can), only after that it
> sets moment->def.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/virdomainmomentobjlist.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

Fixes: 0baf6945ed "snapshot: Minor cleanup to
virDomainSnapshotAssignDef", which I wrote in response to John's good
suggestion, but where I did not pay attention to the fact that the 4
lines in between the one line that I moved dereferenced the same
variable. 'make check' did not catch it; so as penance, I'll be adding
something to the testsuite today using the test driver that would have
caught it.

> 
> diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
> index b2122e7292..01e50ae1d4 100644
> --- a/src/conf/virdomainmomentobjlist.c
> +++ b/src/conf/virdomainmomentobjlist.c
> @@ -227,7 +227,7 @@ virDomainMomentAssignDef(virDomainMomentObjListPtr moments,
>      if (!(moment = virDomainMomentObjNew()))
>          return NULL;
>  
> -    if (virHashAddEntry(moments->objs, moment->def->name, moment) < 0) {
> +    if (virHashAddEntry(moments->objs, def->name, moment) < 0) {
>          VIR_FREE(moment);
>          return NULL;
>      }
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virDomainMomentAssignDef: Don't dereference a NULL pointer

Posted by Daniel P. Berrangé 12 weeks ago
On Fri, Mar 22, 2019 at 10:42:44AM +0100, Michal Privoznik wrote:
> This functions tries to add a domain moment (love the name!) onto
> a list of domain moments. Firstly, it checks if another moment
> with the same name already exists. Then, it creates an empty
> moment (without initializing its definition) and tries to add the
> moment onto the list dereferencing moment definition in that
> process. If it succeeds (which it never can), only after that it
> sets moment->def.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/virdomainmomentobjlist.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list