On Tue, Jul 03, 2018 at 04:22 PM +0200, Peter Krempa <pkrempa@xxxxxxxxxx> wrote: > On Tue, Jul 03, 2018 at 09:34:56 -0400, John Ferlan wrote: >> >> >> On 07/03/2018 08:05 AM, Peter Krempa wrote: >> > The private data of a virStorageSource which is backing an iSCSI hostdev >> > may be NULL if no authentication is present. The code handling the >> > hotplug would attempt to extract the authentication info stored in >> > 'secinfo' without checking if it is allocated which resulted in a crash. >> > >> > Here we opt the easy way to check if srcPriv is not NULL so that we >> > don't duplicate all the logic which selects whether the disk source has >> > a secret. >> > >> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1597550 >> > >> > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> >> > --- >> > src/qemu/qemu_hotplug.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> >> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> >> >> John >> >> Feels like we've done this before... >> >> 97202988 <sigh> and 6050affb7f > > Specifically the former commit was not good enough to cover this case > despite touching this code. Would it make sense to always allocate the qemuDomainStorageSourcePrivate struct? We’d be wasting some memory, but at the same time the code would be easier to read and less prone to error. e.g. qemuDomainDeviceDiskDefPostParseRestoreSecAlias would be simpler. For the privateData of virConnect, virDomainVsockDef, virDisk, virDomainVcpu… it’s implemented this way. Anyway: Reviewed-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx> and thanks for fixing it! > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
