On Wed, Sep 20, 2017 at 10:41:49AM -0400, John Ferlan wrote: > > > On 09/20/2017 09:11 AM, Erik Skultety wrote: > > On Wed, Sep 20, 2017 at 05:32:29AM -0700, Ashish Mittal wrote: > >> Passing a NULL value for the argument secAlias to the function > >> qemuDomainGetTLSObjects would cause a segmentation fault in > >> libvirtd. > >> > >> Changed code to not dereference a NULL secAlias. > >> > >> Signed-off-by: Ashish Mittal <ashmit602@xxxxxxxxx> > >> --- > >> 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))) > > > > A few lines above this context, we check whether we have a valid reference to > > @secinfo and if so, we try to fill the @secAlias. Can we guarantee that when > > @secinfo is passed, @secAlias has been set as well? I'm asking because I > > haven't touched the TLS code yet and I just ran a few argument combinations > > mentally and this one got me wondering. If the combination I'm describing is a > > pure non-sense, the patch can go straight in. > > > > True, right, ugh. A result of multiple rewrites along the way in patch > review resulting in me missing something. > > In the case of the Veritas series, it won't matter because secinfo will > be NULL, but that condition should be > > if (secinfo && tlsAlias) tlsAlias?? I thought it should be @secAlias, since that's the one we're setting, can you elaborate more? I'm not familiar with our TLS code so I'd like to learn something :). Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
