On 11/02/2011 10:25 AM, Wen Congyang wrote:
On 11/02/2011 07:16 PM, Stefan Berger wrote:
On 11/02/2011 01:17 AM, 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.
---
src/util/bridge.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/util/bridge.c b/src/util/bridge.c
index 952f0f3..6774f99 100644
--- a/src/util/bridge.c
+++ b/src/util/bridge.c
@@ -484,7 +484,8 @@ brAddTap(brControl *ctl,
errno = brCreateTap(ctl, ifname, vnet_hdr, tapfd);
- if (*tapfd< 0 || errno)
+ /* fd has been closed in brCreateTap() when it failed. */
+ if (errno)
goto error;
/* We need to set the interface MAC before adding it
@@ -494,22 +495,23 @@ brAddTap(brControl *ctl,
* device before we set our static MAC.
*/
if ((errno = brSetInterfaceMac(ctl, *ifname, macaddr)))
- goto error;
+ goto close_fd;
/* 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);
It's not necessary to test for tapfd. This is similar to VIR_FREE().
Besides that you should test for if(tapfd>=0) if testing at all.
Without this check, libvirt may crash. tapfd is pointer, and it is not
necessary to check if *tapfd >= 0, but it is necessary to test for tapfd.
So I pushed this patch without any change.
You're right..
Stefan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list