network/tuntap: manage tun/tap fds by manager

Otherwise, when a .netdev file for tun or tap netdev is updated,
reloading the file leaks the previous file descriptor.

2 files changed, 71 insertions, 63 deletions

diff --git a/src/network/netdev/tuntap.c b/src/network/netdev/tuntap.c
index c012bc1591..a421e66733 100644
--- a/src/network/netdev/tuntap.c
+++ b/src/network/netdev/tuntap.c
@@ -36,23 +36,15 @@ static TunTap* TUNTAP(NetDev *netdev) {
 
 DEFINE_PRIVATE_HASH_OPS_FULL(named_fd_hash_ops, char, string_hash_func, string_compare_func, free, void, close_fd_ptr);
 
-int manager_add_tuntap_fd(Manager *m, int fd, const char *name) {
+static int manager_add_tuntap_fd_impl(Manager *m, int fd, const char *name) {
         _cleanup_free_ char *tuntap_name = NULL;
-        const char *p;
         int r;
 
         assert(m);
         assert(fd >= 0);
         assert(name);
 
-        p = startswith(name, "tuntap-");
-        if (!p)
-                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received unknown fd (%s).", name);
-
-        if (!ifname_valid(p))
-                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received tuntap fd with invalid name (%s).", p);
-
-        tuntap_name = strdup(p);
+        tuntap_name = strdup(name);
         if (!tuntap_name)
                 return log_oom_debug();
 
@@ -64,54 +56,90 @@ int manager_add_tuntap_fd(Manager *m, int fd, const char *name) {
         return 0;
 }
 
-void manager_clear_unmanaged_tuntap_fds(Manager *m) {
-        char *name;
-        void *p;
+int manager_add_tuntap_fd(Manager *m, int fd, const char *name) {
+        const char *p;
 
         assert(m);
+        assert(fd >= 0);
+        assert(name);
 
-        while ((p = hashmap_steal_first_key_and_value(m->tuntap_fds_by_name, (void**) &name))) {
-                close_and_notify_warn(PTR_TO_FD(p), name);
-                name = mfree(name);
-        }
+        p = startswith(name, "tuntap-");
+        if (!p)
+                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received unknown fd (%s).", name);
+
+        if (!ifname_valid(p))
+                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received tuntap fd with invalid name (%s).", p);
+
+        return manager_add_tuntap_fd_impl(m, fd, p);
 }
 
-static int tuntap_take_fd(NetDev *netdev) {
-        _cleanup_free_ char *name = NULL;
+static int netdev_take_tuntap_fd(Manager *m, const char *ifname) {
+        _unused_ _cleanup_free_ char *name = NULL;
         void *p;
+
+        assert(m);
+        assert(ifname);
+
+        p = hashmap_remove2(m->tuntap_fds_by_name, ifname, (void**) &name);
+        if (!p)
+                return -EBADF;
+
+        return PTR_TO_FD(p);
+}
+
+static int netdev_push_tuntap_fd(NetDev *netdev, int fd) {
+        _unused_ _cleanup_close_ int fd_old = -EBADF;
         int r;
 
-        assert(netdev);
         assert(netdev->manager);
 
-        r = link_get_by_name(netdev->manager, netdev->ifname, NULL);
+        fd_old = netdev_take_tuntap_fd(netdev->manager, netdev->ifname);
+
+        if (!TUNTAP(netdev)->keep_fd)
+                return 0;
+
+        r = manager_add_tuntap_fd_impl(netdev->manager, fd, netdev->ifname);
         if (r < 0)
                 return r;
 
-        p = hashmap_remove2(netdev->manager->tuntap_fds_by_name, netdev->ifname, (void**) &name);
-        if (!p)
-                return -ENOENT;
+        (void) notify_push_fdf(fd, "tuntap-%s", netdev->ifname);
+        return 1; /* saved */
+}
 
-        log_netdev_debug(netdev, "Found file descriptor in fd store.");
-        return PTR_TO_FD(p);
+static void manager_close_and_notify_tuntap_fd(Manager *m, const char *ifname) {
+        assert(m);
+        assert(ifname);
+
+        /* netdev_take_tuntap_fd() may invalidate ifname. Hence, need to create fdname earlier. */
+        const char *fdname = strjoina("tuntap-", ifname);
+        close_and_notify_warn(netdev_take_tuntap_fd(m, ifname), fdname);
+}
+
+void manager_clear_unmanaged_tuntap_fds(Manager *m) {
+        const char *name;
+        void *p;
+
+        assert(m);
+
+        HASHMAP_FOREACH_KEY(p, name, m->tuntap_fds_by_name) {
+                NetDev *netdev;
+
+                if (netdev_get(m, name, &netdev) < 0 ||
+                    !IN_SET(netdev->kind, NETDEV_KIND_TAP, NETDEV_KIND_TUN) ||
+                    !TUNTAP(netdev)->keep_fd)
+                        manager_close_and_notify_tuntap_fd(m, name);
+        }
 }
 
 static int netdev_create_tuntap(NetDev *netdev) {
         _cleanup_close_ int fd = -EBADF;
         struct ifreq ifr = {};
-        TunTap *t;
+        TunTap *t = TUNTAP(netdev);
         int r;
 
-        assert(netdev);
         assert(netdev->manager);
-        t = TUNTAP(netdev);
-        assert(t);
 
-        fd = TAKE_FD(t->fd);
-        if (fd < 0)
-                fd = tuntap_take_fd(netdev);
-        if (fd < 0)
-                fd = open(TUN_DEV, O_RDWR|O_CLOEXEC);
+        fd = open(TUN_DEV, O_RDWR|O_CLOEXEC);
         if (fd < 0)
                 return log_netdev_error_errno(netdev, errno,  "Failed to open " TUN_DEV ": %m");
 
@@ -175,42 +203,25 @@ static int netdev_create_tuntap(NetDev *netdev) {
         if (ioctl(fd, TUNSETPERSIST, 1) < 0)
                 return log_netdev_error_errno(netdev, errno, "TUNSETPERSIST failed: %m");
 
-        if (t->keep_fd) {
-                t->fd = TAKE_FD(fd);
-                (void) notify_push_fdf(t->fd, "tuntap-%s", netdev->ifname);
-        }
+        r = netdev_push_tuntap_fd(netdev, fd);
+        if (r < 0)
+                return log_netdev_warning_errno(netdev, r, "Failed to save TUN/TAP fd: %m");
+        if (r > 0)
+                TAKE_FD(fd);
 
+        netdev_enter_ready(netdev);
         return 0;
 }
 
-static void tuntap_init(NetDev *netdev) {
-        TunTap *t;
-
-        assert(netdev);
-        t = TUNTAP(netdev);
-        assert(t);
-
-        t->fd = -EBADF;
-}
-
 static void tuntap_drop(NetDev *netdev) {
-        TunTap *t;
-
         assert(netdev);
-        t = TUNTAP(netdev);
-        assert(t);
 
-        t->fd = close_and_notify_warn(t->fd, netdev->ifname);
+        manager_close_and_notify_tuntap_fd(netdev->manager, netdev->ifname);
 }
 
 static void tuntap_done(NetDev *netdev) {
-        TunTap *t;
-
-        assert(netdev);
-        t = TUNTAP(netdev);
-        assert(t);
+        TunTap *t = TUNTAP(netdev);
 
-        t->fd = safe_close(t->fd);
         t->user_name = mfree(t->user_name);
         t->group_name = mfree(t->group_name);
 }
@@ -237,7 +248,6 @@ const NetDevVTable tun_vtable = {
         .object_size = sizeof(TunTap),
         .sections = NETDEV_COMMON_SECTIONS "Tun\0",
         .config_verify = tuntap_verify,
-        .init = tuntap_init,
         .drop = tuntap_drop,
         .done = tuntap_done,
         .create = netdev_create_tuntap,
@@ -249,7 +259,6 @@ const NetDevVTable tap_vtable = {
         .object_size = sizeof(TunTap),
         .sections = NETDEV_COMMON_SECTIONS "Tap\0",
         .config_verify = tuntap_verify,
-        .init = tuntap_init,
         .drop = tuntap_drop,
         .done = tuntap_done,
         .create = netdev_create_tuntap,
diff --git a/src/network/netdev/tuntap.h b/src/network/netdev/tuntap.h
index 88e0ce5f97..e2de8eb6bb 100644
--- a/src/network/netdev/tuntap.h
+++ b/src/network/netdev/tuntap.h
@@ -8,7 +8,6 @@ typedef struct TunTap TunTap;
 struct TunTap {
         NetDev meta;
 
-        int fd;
         char *user_name;
         char *group_name;
         bool multi_queue;