On 05/10/2016 02:52 PM, John Ferlan wrote:

Remove the possibility that a NULL hostdev->privateData or a disk->privateData could crash libvirtd by checking for NULL before dereferencing for the secinfo structure in the qemuDomainSecret{Disk|Hostdev}Destroy functions. The hostdevPriv could be NULL if qemuProcessNetworkPrepareDevices adds a new hostdev during virDomainNetGetActualHostdev that then gets inserted via virDomainHostdevInsert. The hostdevPriv was added by commit id '27726d8' and is currently only used by scsi hostdev.

Signed-off-by: John Ferlan <jferlan@redhat.com> ---

Discovered by laine and debugged on private IRC channel.

src/qemu/qemu_domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 93f0a01..0cddb86 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -903,7 +903,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) { qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);

- if (!diskPriv->secinfo) + if (!diskPriv || !diskPriv->secinfo) return;

qemuDomainSecretInfoFree(&diskPriv->secinfo);

I was thinking maybe this instead: if (diskPriv && diskPriv->secinfo) qemuDomainSecretInfoFree(&diskPriv->secinfo); but yours works too. Either way, ACK.

@@ -964,7 +964,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev) qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev);

- if (!hostdevPriv->secinfo) + if (!hostdevPriv || !hostdevPriv->secinfo) return;

qemuDomainSecretInfoFree(&hostdevPriv->secinfo);