* [PATCH] net/tap-solaris: Fix resource leaks on error paths
@ 2026-07-24 9:38 Weimin Xiong
2026-09-04 18:02 ` Michael S. Tsirkin
0 siblings, 1 reply; 2+ messages in thread
From: Weimin Xiong @ 2026-07-24 9:38 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, mst, qemu-trivial, Xiong Weimin
From: Xiong Weimin <xiongweimin@kylinos.cn>
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 <xiongweimin@kylinos.cn>
---
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;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] net/tap-solaris: Fix resource leaks on error paths
2026-07-24 9:38 [PATCH] net/tap-solaris: Fix resource leaks on error paths Weimin Xiong
@ 2026-09-04 18:02 ` Michael S. Tsirkin
0 siblings, 0 replies; 2+ messages in thread
From: Michael S. Tsirkin @ 2026-09-04 18:02 UTC (permalink / raw)
To: Weimin Xiong; +Cc: qemu-devel, jasowang, qemu-trivial, Xiong Weimin
On Fri, Jul 24, 2026 at 05:38:09PM +0800, Weimin Xiong wrote:
> From: Xiong Weimin <xiongweimin@kylinos.cn>
>
> 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 <xiongweimin@kylinos.cn>
> ---
> 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
mode 4321006? this line seems corrupt, at least it confuses my git am
> --- 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);
so you close ip_fd here ...
>
> 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) {
and then try to do an ioctl on it?
> + 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);
arp_fd closed here
> +fail_ip_muxid:
> + ioctl(ip_fd, I_PUNLINK, ip_muxid);
> +fail_arp_fd:
> + close(arp_fd);
then again here
> +fail_if_fd:
> + close(if_fd);
> +fail_tap_fd:
> + close(tap_fd);
> + if (ip_fd > 0) {
> + close(ip_fd);
> + ip_fd = 0;
> + }
> + return -1;
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-04 18:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 9:38 [PATCH] net/tap-solaris: Fix resource leaks on error paths Weimin Xiong
2026-09-04 18:02 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.