1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] Fix libvirtd crash in qemuDomainGetTLSObjects

Ashish Mittal posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1505883514-26215-1-git-send-email-ashmit602@gmail.com
There is a newer version of this series
src/qemu/qemu_hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[libvirt] [PATCH] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by Ashish Mittal 6 years, 7 months ago 
Passing a NULL value for the argument secAlias to the function
qemuDomainGetTLSObjects causes a segmentation fault.

Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault.
0x00007f97c9c42a3d in qemuDomainGetTLSObjects (...,secAlias=0x0)
at qemu/qemu_hotplug.c:1736

Changed code to not dereference a NULL secAlias.

Signed-off-by: Ashish Mittal <ashmit602@gmail.com>
---
 src/qemu/qemu_hotplug.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7dd6e5f..df58ecc 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1643,7 +1643,8 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
     }
 
     if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
-                                     *secAlias, qemuCaps, tlsProps) < 0)
+                                     **secAlias ? *secAlias : NULL, qemuCaps,
+                                     tlsProps) < 0)
         return -1;
 
     if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias)))
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by Erik Skultety 6 years, 7 months ago 
On Tue, Sep 19, 2017 at 09:58:34PM -0700, Ashish Mittal wrote:
> Passing a NULL value for the argument secAlias to the function
> qemuDomainGetTLSObjects causes a segmentation fault.
>
> Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault.
> 0x00007f97c9c42a3d in qemuDomainGetTLSObjects (...,secAlias=0x0)
> at qemu/qemu_hotplug.c:1736

Can you provide the whole backtrace? Because from what I see in the code,
qemuDomainGetTLSObjects is called from qemu_hotplug.c and qemu_migration.c, but
none of the code paths would result in qemuDomainGetTLSObjects to get secAlias
== NULL, solely because all the callers (direct or indirect) of this method call
it as &secAlias. Therefore, I think the case you're trying to fix cannot
happen in the current state - the fix is also wrong, see below.


>
>      if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
> -                                     *secAlias, qemuCaps, tlsProps) < 0)
> +                                     **secAlias ? *secAlias : NULL, qemuCaps,

So, hypothetically, if secAlias == NULL and *secAlias results in a SEGFAULT,
what is the result of doing **secAlias? Correct, a SEGFAULT.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by Erik Skultety 6 years, 7 months ago 
On Wed, Sep 20, 2017 at 09:03:18AM +0200, Erik Skultety wrote:
> On Tue, Sep 19, 2017 at 09:58:34PM -0700, Ashish Mittal wrote:
> > Passing a NULL value for the argument secAlias to the function
> > qemuDomainGetTLSObjects causes a segmentation fault.
> >
> > Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault.
> > 0x00007f97c9c42a3d in qemuDomainGetTLSObjects (...,secAlias=0x0)
> > at qemu/qemu_hotplug.c:1736
>
> Can you provide the whole backtrace? Because from what I see in the code,
> qemuDomainGetTLSObjects is called from qemu_hotplug.c and qemu_migration.c, but
> none of the code paths would result in qemuDomainGetTLSObjects to get secAlias
> == NULL, solely because all the callers (direct or indirect) of this method call

Oh, I see, this is supposed to be a follow-up patch to
https://www.redhat.com/archives/libvir-list/2017-September/msg00645.html.
You can disregard my comment above then, the fix still needs to be adjusted
though as pointed out in my previous response.

Erik

> it as &secAlias. Therefore, I think the case you're trying to fix cannot
> happen in the current state - the fix is also wrong, see below.
>
>
> >
> >      if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
> > -                                     *secAlias, qemuCaps, tlsProps) < 0)
> > +                                     **secAlias ? *secAlias : NULL, qemuCaps,
>
> So, hypothetically, if secAlias == NULL and *secAlias results in a SEGFAULT,
> what is the result of doing **secAlias? Correct, a SEGFAULT.
>
> Erik
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by ashish mittal 6 years, 7 months ago 
On Wed, Sep 20, 2017 at 12:52 AM, Erik Skultety <eskultet@redhat.com> wrote:

> On Wed, Sep 20, 2017 at 09:03:18AM +0200, Erik Skultety wrote:
> > On Tue, Sep 19, 2017 at 09:58:34PM -0700, Ashish Mittal wrote:
> > > Passing a NULL value for the argument secAlias to the function
> > > qemuDomainGetTLSObjects causes a segmentation fault.
> > >
> > > Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault.
> > > 0x00007f97c9c42a3d in qemuDomainGetTLSObjects (...,secAlias=0x0)
> > > at qemu/qemu_hotplug.c:1736
> >
> > Can you provide the whole backtrace? Because from what I see in the code,
> > qemuDomainGetTLSObjects is called from qemu_hotplug.c and
> qemu_migration.c, but
> > none of the code paths would result in qemuDomainGetTLSObjects to get
> secAlias
> > == NULL, solely because all the callers (direct or indirect) of this
> method call
>
> Oh, I see, this is supposed to be a follow-up patch to
> https://www.redhat.com/archives/libvir-list/2017-September/msg00645.html.
> You can disregard my comment above then, the fix still needs to be adjusted
> though as pointed out in my previous response.
>
> Erik
>
>
Thanks for pointing out my blunder! I just realized that I had not
restarted libvirtd service after running "make install" when I tested my
change. The changes appeared to work because I had the original fix
described in
https://www.redhat.com/archives/libvir-list/2017-September/msg00638.html
still in libvirtd.

I will repost after fixing and testing again.

Ashish

> it as &secAlias. Therefore, I think the case you're trying to fix cannot
> > happen in the current state - the fix is also wrong, see below.
> >
> >
> > >
> > >      if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen,
> tlsVerify,
> > > -                                     *secAlias, qemuCaps, tlsProps) <
> 0)
> > > +                                     **secAlias ? *secAlias : NULL,
> qemuCaps,
> >
> > So, hypothetically, if secAlias == NULL and *secAlias results in a
> SEGFAULT,
> > what is the result of doing **secAlias? Correct, a SEGFAULT.
> >
> > Erik
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.