[libvirt] [PATCH v2] Fix libvirtd crash in qemuDomainGetTLSObjects

Ashish Mittal posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1505908956-106770-1-git-send-email-ashmit602@gmail.com
src/qemu/qemu_hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[libvirt] [PATCH v2] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by Ashish Mittal 6 years, 6 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..9ecdf0a 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 v2] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by Erik Skultety 6 years, 6 months ago
On Wed, Sep 20, 2017 at 05:02:36AM -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
>
> Changed code to not dereference a NULL secAlias.
>
> Signed-off-by: Ashish Mittal <ashmit602@gmail.com>

Looks better, but this should IMHO go as part of the larger series, because on
its own, it doesn't make much sense to fix an issue that doesn't exist yet, but
will in a short period of time. Therefore, rather than doing it in a separate
patch, we should make it part of the series, because, repeating myself, this
should be a mere adjustment necessary for the larger series to work properly,
not a fix of an issue - the issue should either exist already or the series
shouldn't introduce a crasher in the first place.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by John Ferlan 6 years, 6 months ago

On 09/20/2017 08:11 AM, Erik Skultety wrote:
> On Wed, Sep 20, 2017 at 05:02:36AM -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
>>
>> Changed code to not dereference a NULL secAlias.
>>
>> Signed-off-by: Ashish Mittal <ashmit602@gmail.com>
> 
> Looks better, but this should IMHO go as part of the larger series, because on
> its own, it doesn't make much sense to fix an issue that doesn't exist yet, but
> will in a short period of time. Therefore, rather than doing it in a separate
> patch, we should make it part of the series, because, repeating myself, this
> should be a mere adjustment necessary for the larger series to work properly,
> not a fix of an issue - the issue should either exist already or the series
> shouldn't introduce a crasher in the first place.
> 
> Erik
> 

I asked for a separate patch although while related to what's changing
for the larger Veritas VxHS series, it is still a bug in the code today
even though we haven't yet hit it because our callers have provided the
secalias.

Perhaps the commit message could be stated "Avoid a possible NULL
dereference on a parameter that is checked for NULL in other places."

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fix libvirtd crash in qemuDomainGetTLSObjects
Posted by Peter Krempa 6 years, 6 months ago
On Wed, Sep 20, 2017 at 08:14:39 -0400, John Ferlan wrote:
> 
> 
> On 09/20/2017 08:11 AM, Erik Skultety wrote:
> > On Wed, Sep 20, 2017 at 05:02:36AM -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
> >>
> >> Changed code to not dereference a NULL secAlias.
> >>
> >> Signed-off-by: Ashish Mittal <ashmit602@gmail.com>
> > 
> > Looks better, but this should IMHO go as part of the larger series, because on
> > its own, it doesn't make much sense to fix an issue that doesn't exist yet, but
> > will in a short period of time. Therefore, rather than doing it in a separate
> > patch, we should make it part of the series, because, repeating myself, this
> > should be a mere adjustment necessary for the larger series to work properly,
> > not a fix of an issue - the issue should either exist already or the series
> > shouldn't introduce a crasher in the first place.
> > 
> > Erik
> > 
> 
> I asked for a separate patch although while related to what's changing
> for the larger Veritas VxHS series, it is still a bug in the code today
> even though we haven't yet hit it because our callers have provided the
> secalias.
> 
> Perhaps the commit message could be stated "Avoid a possible NULL
> dereference on a parameter that is checked for NULL in other places."

Yes. Exactly. The patch should be pushed prior to the code which would
crash, thus the crash will never happen and the backtrace and whole
commit message will be wrong.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list