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

Michal Privoznik posted 1 patch 5 years, 1 month 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 5 years, 1 month 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 Daniel P. Berrangé 5 years, 1 month 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
Re: [libvirt] [PATCH] virDomainMomentAssignDef: Don't dereference a NULL pointer
Posted by Eric Blake 5 years, 1 month 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