[PATCH 0/2] virnetdevtap: fix tap device leaks on FreeBSD
Roman Bogorodskiy (2): virnetdevtap: fix tap device leak on rename virnetdevtap: fix tap device leak in virNetDevTapCreateInBridgePort() src/util/virnetdevtap.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) -- 2.55.0
On FreeBSD, virNetDevTapCreate() creates a generic tap(4) device and then renames to a desired value (typically vnetN). However, it does not delete the tap(4) device on errors, so it leaks the device when rename fails. Fix by attempting to remove the device in "cleanup:" if it was created. It's also closed before that because FreeBSD will not delete an opened device. Error handling is slightly updated: - For the unsupported "tapfdSize > 1" case return immediately, nothing to clean there - Also return when device creation fails. We have to duplicate VIR_FORCE_CLOSE(), but then in "cleanup" we are sure that "ifr" is initialized. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/util/virnetdevtap.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index e3a6209642..aa0907dd0c 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -334,11 +334,12 @@ int virNetDevTapCreate(char **ifname, int s; struct ifreq ifr; int ret = -1; + bool created = false; if (tapfdSize > 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Multiqueue devices are not supported on this system")); - goto cleanup; + return -1; } /* auto-generate an unused name for the new device (this @@ -357,9 +358,12 @@ int virNetDevTapCreate(char **ifname, if (ioctl(s, SIOCIFCREATE2, &ifr) < 0) { virReportSystemError(errno, "%s", _("Unable to create tap device")); - goto cleanup; + VIR_FORCE_CLOSE(s); + return -1; } + created = true; + if (tapfd) { g_autofree char *dev_path = NULL; dev_path = g_strdup_printf("/dev/%s", ifr.ifr_name); @@ -378,6 +382,18 @@ int virNetDevTapCreate(char **ifname, ret = 0; cleanup: + if (ret < 0 && created) { + virErrorPtr err; + + virErrorPreserveLast(&err); + + if (tapfd) + VIR_FORCE_CLOSE(*tapfd); + + ignore_value(virNetDevTapDelete(ifr.ifr_name, NULL)); + virErrorRestore(&err); + } + VIR_FORCE_CLOSE(s); return ret; -- 2.55.0
On FreeBSD, virNetDevTapCreateInBridgePort() might leak a tap(4) device. Specifically, we're creating a tap(4) device with virNetDevTapCreate() early in the function. Then we are performing various operations, e.g. virNetDevSetMAC() and virNetDevTapAttachBridge(), and on errors jump to the 'error' cleanup label where we close the tap device. As tap(4) devices configured by libvirt on FreeBSD do not destroy themselves on closing, they leak on errors in this function. So make sure to add device removal to the error handling of this function. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/util/virnetdevtap.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index aa0907dd0c..6c228ff086 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -695,6 +695,16 @@ int virNetDevTapCreateInBridgePort(const char *brname, for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++) VIR_FORCE_CLOSE(tapfd[i]); +#if defined(VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP) + if (!(flags & VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING)) { + virErrorPtr err; + + virErrorPreserveLast(&err); + ignore_value(virNetDevTapDelete(*ifname, tunpath)); + virErrorRestore(&err); + } +#endif /* VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP */ + return -1; } -- 2.55.0
On 9/9/26 20:22, Roman Bogorodskiy wrote:
Roman Bogorodskiy (2): virnetdevtap: fix tap device leak on rename virnetdevtap: fix tap device leak in virNetDevTapCreateInBridgePort()
src/util/virnetdevtap.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Roman Bogorodskiy