[libvirt] [PATCH] audit: eliminate potential null pointer deref when auditing macvtap devices
The newly added call to qemuAuditNetDevice in qemuPhysIfaceConnect was assuming that res_ifname (the name of the macvtap device) was always valid, but this isn't the case. If openMacvtapTap fails, it always returns NULL, which would result in a segv. Since the audit log only needs a record of devices that are actually sent to qemu, and a failure to open the macvtap device means that no device will be sent to qemu, we can solve this problem by only doing the audit if openMacvtapTap is successful (in which case res_ifname is guaranteed valid). --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8ca656a..72f55d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -128,8 +128,8 @@ qemuPhysIfaceConnect(virDomainDefPtr def, net->data.direct.mode, vnet_hdr, def->uuid, &net->data.direct.virtPortProfile, &res_ifname, vmop); - qemuAuditNetDevice(def, net, res_ifname, rc >= 0); if (rc >= 0) { + qemuAuditNetDevice(def, net, res_ifname, true); VIR_FREE(net->ifname); net->ifname = res_ifname; } -- 1.7.3.4
On 03/14/2011 09:18 AM, Laine Stump wrote:
The newly added call to qemuAuditNetDevice in qemuPhysIfaceConnect was assuming that res_ifname (the name of the macvtap device) was always valid, but this isn't the case. If openMacvtapTap fails, it always returns NULL, which would result in a segv.
Since the audit log only needs a record of devices that are actually sent to qemu, and a failure to open the macvtap device means that no device will be sent to qemu, we can solve this problem by only doing the audit if openMacvtapTap is successful (in which case res_ifname is guaranteed valid). --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8ca656a..72f55d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -128,8 +128,8 @@ qemuPhysIfaceConnect(virDomainDefPtr def, net->data.direct.mode, vnet_hdr, def->uuid, &net->data.direct.virtPortProfile, &res_ifname, vmop); - qemuAuditNetDevice(def, net, res_ifname, rc >= 0); if (rc >= 0) { + qemuAuditNetDevice(def, net, res_ifname, true); VIR_FREE(net->ifname); net->ifname = res_ifname; }
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 03/14/2011 11:25 AM, Eric Blake wrote:
On 03/14/2011 09:18 AM, Laine Stump wrote:
The newly added call to qemuAuditNetDevice in qemuPhysIfaceConnect was assuming that res_ifname (the name of the macvtap device) was always valid, but this isn't the case. If openMacvtapTap fails, it always returns NULL, which would result in a segv.
Since the audit log only needs a record of devices that are actually sent to qemu, and a failure to open the macvtap device means that no device will be sent to qemu, we can solve this problem by only doing the audit if openMacvtapTap is successful (in which case res_ifname is guaranteed valid). --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8ca656a..72f55d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -128,8 +128,8 @@ qemuPhysIfaceConnect(virDomainDefPtr def, net->data.direct.mode, vnet_hdr, def->uuid, &net->data.direct.virtPortProfile,&res_ifname, vmop); - qemuAuditNetDevice(def, net, res_ifname, rc>= 0); if (rc>= 0) { + qemuAuditNetDevice(def, net, res_ifname, true); VIR_FREE(net->ifname); net->ifname = res_ifname; } ACK.
Pushed. Thanks!
participants (2)
-
Eric Blake -
Laine Stump