On 11/01/2011 11:17 PM, Wen Congyang wrote:
commit 27908453 introduces a regression, and it will cause libvirt crashed when starting network. The reason is that tapfd may be NULL, but we dereference it without checking whether it is NULL.
if ((errno = brSetInterfaceMac(ctl, *ifname, macaddr))) - goto error; + goto close_fd;
I don't think we needed the new label...
/* We need to set the interface MTU before adding it * to the bridge, because the bridge will have its * MTU adjusted automatically when we add the new interface. */ if ((errno = brSetInterfaceMtu(ctl, bridge, *ifname))) - goto error; + goto close_fd; if ((errno = brAddInterface(ctl, bridge, *ifname))) - goto error; + goto close_fd; if (up&& ((errno = brSetInterfaceUp(ctl, *ifname, 1)))) - goto error; + goto close_fd; return 0; - error: - VIR_FORCE_CLOSE(*tapfd); - +close_fd: + if (tapfd) + VIR_FORCE_CLOSE(*tapfd);
That is, just the added if(tapfd) was sufficient to fix the bug, and the old code was still correct in trying to close the fd on all error paths with one less label.
But since you've already pushed, we can keep it as is; it's not worth the churn to cut out the new label.
-- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list