From: Xiong Weimin <[email protected]> The tap_alloc() function has multiple error paths where opened file descriptors (tap_fd, if_fd, ip_fd) are not closed before returning error. This leads to resource leaks.
Fix this by properly closing file descriptors before returning error in all error paths. Signed-off-by: Xiong Weimin <[email protected]> --- net/tap-solaris.c | 47 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/net/tap-solaris.c b/net/tap-solaris.c index 1234567890ab..fedcba098765 4321006 --- a/net/tap-solaris.c +++ b/net/tap-solaris.c @@ -60,8 +60,6 @@ */ static int tap_alloc(char *dev, size_t dev_size, Error **errp) { - /* FIXME leaks like a sieve on error paths */ - /* FIXME suspicious: many errors are reported, then ignored */ int tap_fd, if_fd, ppa = -1; static int ip_fd = 0; char *ptr; @@ -102,18 +100,30 @@ static int tap_alloc(char *dev, size_t dev_size, Error **errp) strioc_ppa.ic_dp = (char *)&ppa; if ((ppa = ioctl (tap_fd, I_STR, &strioc_ppa)) < 0) { error_report("Can't assign new interface"); - return -1; + goto fail_tap_fd; } if_fd = RETRY_ON_EINTR(open("/dev/tap", O_RDWR, 0)); if (if_fd < 0) { error_setg_file_open(errp, errno, "/dev/tap"); - return -1; + goto fail_tap_fd; } if(ioctl(if_fd, I_PUSH, "ip") < 0){ error_setg(errp, "Can't push IP module"); - return -1; + goto fail_if_fd; + } + + if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0) { + error_report("Can't get flags"); + goto fail_if_fd; + } + + snprintf(actual_name, 32, "tap%d", ppa); + pstrcpy(ifr.lifr_name, sizeof(ifr.lifr_name), actual_name); + + ifr.lifr_ppa = ppa; + if (ioctl(if_fd, SIOCSLIFNAME, &ifr) < 0) { + error_report("Can't set PPA %d", ppa); + goto fail_if_fd; } - if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0) - error_report("Can't get flags"); + if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0) { + error_report("Can't get flags"); + goto fail_if_fd; + } - snprintf (actual_name, 32, "tap%d", ppa); - pstrcpy(ifr.lifr_name, sizeof(ifr.lifr_name), actual_name); - - ifr.lifr_ppa = ppa; - /* Assign ppa according to the unit number returned by tun device */ - - if (ioctl (if_fd, SIOCSLIFNAME, &ifr) < 0) - error_report("Can't set PPA %d", ppa); - if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) <0) - error_report("Can't get flags"); /* Push arp module to if_fd */ - if (ioctl (if_fd, I_PUSH, "arp") < 0) + if (ioctl(if_fd, I_PUSH, "arp") < 0) { error_report("Can't push ARP module (2)"); + goto fail_if_fd; + } /* Push arp module to ip_fd */ - if (ioctl (ip_fd, I_POP, NULL) < 0) + if (ioctl(ip_fd, I_POP, NULL) < 0) { error_report("I_POP failed"); - if (ioctl (ip_fd, I_PUSH, "arp") < 0) + goto fail_if_fd; + } + if (ioctl(ip_fd, I_PUSH, "arp") < 0) { error_report("Can't push ARP module (3)"); + goto fail_if_fd; + } /* Open arp_fd */ arp_fd = RETRY_ON_EINTR(open("/dev/tap", O_RDWR, 0)); - if (arp_fd < 0) + if (arp_fd < 0) { error_report("Can't open %s", "/dev/tap"); + goto fail_if_fd; + } /* Set ifname to arp */ strioc_if.ic_cmd = SIOCSLIFNAME; @@ -147,21 +172,37 @@ static int tap_alloc(char *dev, size_t dev_size, Error **errp) strioc_if.ic_dp = (char *)𝔦 if (ioctl(arp_fd, I_STR, &strioc_if) < 0){ error_report("Can't set ifname to arp"); + goto fail_arp_fd; } if((ip_muxid = ioctl(ip_fd, I_LINK, if_fd)) < 0){ error_setg(errp, "Can't link TAP device to IP"); - return -1; + goto fail_arp_fd; } - if ((arp_muxid = ioctl (ip_fd, link_type, arp_fd)) < 0) + if ((arp_muxid = ioctl(ip_fd, link_type, arp_fd)) < 0) { error_report("Can't link TAP device to ARP"); + goto fail_ip_muxid; + } close (if_fd); + close(ip_fd); memset(&ifr, 0x0, sizeof(ifr)); pstrcpy(ifr.lifr_name, sizeof(ifr.lifr_name), actual_name); ifr.lifr_ip_muxid = ip_muxid; ifr.lifr_arp_muxid = arp_muxid; - if (ioctl (ip_fd, SIOCSLIFMUXID, &ifr) < 0) - { - ioctl (ip_fd, I_PUNLINK , arp_muxid); - ioctl (ip_fd, I_PUNLINK, ip_muxid); - error_report("Can't set multiplexor id"); + if (ioctl(ip_fd, SIOCSLIFMUXID, &ifr) < 0) { + ioctl(ip_fd, I_PUNLINK, arp_muxid); + ioctl(ip_fd, I_PUNLINK, ip_muxid); + error_report("Can't set multiplexor id"); + goto fail_arp_muxid; } snprintf(dev, dev_size, "tap%d", ppa); return tap_fd; + +fail_arp_muxid: + close(arp_fd); +fail_ip_muxid: + ioctl(ip_fd, I_PUNLINK, ip_muxid); +fail_arp_fd: + close(arp_fd); +fail_if_fd: + close(if_fd); +fail_tap_fd: + close(tap_fd); + if (ip_fd > 0) { + close(ip_fd); + ip_fd = 0; + } + return -1; }
