net/tap-solaris.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
The tap_alloc() function has multiple error paths where opened file
descriptors (tap_fd, if_fd, ip_fd, arp_fd) and STREAMS multiplexors
(ip_muxid, arp_muxid) are not released before returning error. This
leads to resource leaks and, on subsequent calls to tap_alloc(),
to use-after-free because ip_fd is a function-static variable.
Fix this by introducing explicit cleanup labels and releasing all
resources on every error path, in reverse order of acquisition:
fail_ip_muxid: I_PUNLINK ip_muxid
fail_arp_fd: close(arp_fd)
fail_if_fd: close(if_fd)
fail_tap_fd: close(tap_fd); close(ip_fd) if it was opened
The SIOCSLIFMUXID failure path keeps its existing inline
I_PUNLINK arp_muxid / I_PUNLINK ip_muxid calls (those muxids were
both successfully established at that point) and then jumps
directly to fail_if_fd to skip the redundant muxid cleanup.
The ip_fd reset to 0 after close is intentional: ip_fd is a
function-static variable, so any subsequent tap_alloc() invocation
must not see a stale value, otherwise the `if (ip_fd) close(ip_fd);`
at the top of the function would close a fd that some other code
now owns.
Changes since v1:
- Replaced the mixed fall-through / explicit-goto label scheme
of v1 with a single descending goto chain at the bottom of
the function: each label only releases its own resource and
falls through to the next, matching the goto-cleanup pattern
used in net/tap-linux.c.
- The ip_fd close that v1 inserted right before SIOCSLIFMUXID
(which used a stale fd) has been removed. ip_fd is now only
closed in the failure paths.
- arp_muxid and ip_muxid are I_PUNLINK'd exactly once each.
v1's separate fail_arp_muxid label would have double-released
them when the SIOCSLIFMUXID failure path fell through to it.
- This v2 is generated with `git diff` against a fresh checkout
of net/tap-solaris.c, so the file mode and hunks are well-
formed and `git apply --check` passes cleanly. v1's file mode
line was corrupted.
This refactor has not been tested on a real Solaris host, because
I do not have access to one. Please flag anything that looks off.
Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
---
net/tap-solaris.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 8704b10..7ad111f 100644
--- a/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -149,15 +149,18 @@ 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);
@@ -171,10 +174,25 @@ static int tap_alloc(char *dev, size_t dev_size, Error **errp)
ioctl (ip_fd, I_PUNLINK , arp_muxid);
ioctl (ip_fd, I_PUNLINK, ip_muxid);
error_report("Can't set multiplexor id");
+ goto fail_if_fd;
}
snprintf(dev, dev_size, "tap%d", ppa);
return tap_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;
}
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
© 2016 - 2026 Red Hat, Inc.