networkd: refactor link_add() ...

Don't set set **ret when returning r < 0, as matching on the errno may easily
give false positives in the future leading to null pointer dereference.

Reported-by: David Herrmann <dh.herrmann@gmail.com>

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index ec283d5b7b3e9e5c0172c576c747e93ba793e64d..66bcb6bacf051f9ba1afc3381f3c2b3bd25b2fa5 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -85,22 +85,33 @@ void link_free(Link *link) {
         free(link);
 }
 
         free(link);
 }
 

+int link_get(Manager *m, int ifindex, Link **ret) {
+        Link *link;
+        uint64_t ifindex_64;
+
+        assert(m);
+        assert(m->links);
+        assert(ifindex);
+        assert(ret);
+
+        ifindex_64 = ifindex;
+        link = hashmap_get(m->links, &ifindex_64);
+        if (!link)
+                return -ENODEV;
+
+        *ret = link;
+
+        return 0;
+}
+

 int link_add(Manager *m, struct udev_device *device, Link **ret) {
         Link *link;
         Network *network;
         int r;
 int link_add(Manager *m, struct udev_device *device, Link **ret) {
         Link *link;
         Network *network;
         int r;

-        uint64_t ifindex;

 
         assert(m);
         assert(device);
 
 
         assert(m);
         assert(device);
 

-        ifindex = udev_device_get_ifindex(device);
-        link = hashmap_get(m->links, &ifindex);
-        if (link) {
-                *ret = link;
-                return -EEXIST;
-        }
-

         r = link_new(m, device, &link);
         if (r < 0)
                 return r;
         r = link_new(m, device, &link);
         if (r < 0)
                 return r;


@@ -926,7 +937,8 @@ static int link_enter_enslave(Link *link) {
         return 0;
 }
 
         return 0;
 }
 

-static int link_get_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
+static int link_getlink_handler(sd_rtnl *rtnl, sd_rtnl_message *m,
+                                void *userdata) {

         Link *link = userdata;
         int r;
 
         Link *link = userdata;
         int r;
 


@@ -953,7 +965,7 @@ static int link_get_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
         return 1;
 }
 
         return 1;
 }
 

-static int link_get(Link *link) {
+static int link_getlink(Link *link) {

         _cleanup_rtnl_message_unref_ sd_rtnl_message *req = NULL;
         int r;
 
         _cleanup_rtnl_message_unref_ sd_rtnl_message *req = NULL;
         int r;
 


@@ -970,7 +982,8 @@ static int link_get(Link *link) {
                 return r;
         }
 
                 return r;
         }
 

-        r = sd_rtnl_call_async(link->manager->rtnl, req, link_get_handler, link, 0, NULL);
+        r = sd_rtnl_call_async(link->manager->rtnl, req, link_getlink_handler,
+                               link, 0, NULL);

         if (r < 0) {
                 log_error_link(link,
                                "Could not send rtnetlink message: %s", strerror(-r));
         if (r < 0) {
                 log_error_link(link,
                                "Could not send rtnetlink message: %s", strerror(-r));


@@ -987,7 +1000,7 @@ int link_configure(Link *link) {
         assert(link->network);
         assert(link->state == _LINK_STATE_INVALID);
 
         assert(link->network);
         assert(link->state == _LINK_STATE_INVALID);
 

-        r = link_get(link);
+        r = link_getlink(link);

         if (r < 0) {
                 link_enter_failed(link);
                 return r;
         if (r < 0) {
                 link_enter_failed(link);
                 return r;

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index c1ad69bf514295614654760642242c609ede98fc..f817d698c4cfec2ad0956886c82d8b70a0ee7ad6 100644 (file)
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -145,30 +145,31 @@ bool manager_should_reload(Manager *m) {
 }
 
 static int manager_process_link(Manager *m, struct udev_device *device) {
 }
 
 static int manager_process_link(Manager *m, struct udev_device *device) {

-        Link *link;
+        Link *link = NULL;

         int r;
 
         int r;
 

-        if (streq_ptr(udev_device_get_action(device), "remove")) {
-                uint64_t ifindex;
+        assert(m);
+        assert(device);

 
 

+        link_get(m, udev_device_get_ifindex(device), &link);
+
+        if (streq_ptr(udev_device_get_action(device), "remove")) {

                 log_debug("%s: link removed", udev_device_get_sysname(device));
 
                 log_debug("%s: link removed", udev_device_get_sysname(device));
 

-                ifindex = udev_device_get_ifindex(device);
-                link = hashmap_get(m->links, &ifindex);
-                if (!link)
+                if (link)
+                        link_free(link);
+        } else {
+                if (link) {
+                        log_debug("%s: link already exists, ignoring",
+                                  link->ifname);

                         return 0;
                         return 0;

+                }

 
 

-                link_free(link);
-        } else {

                 r = link_add(m, device, &link);
                 if (r < 0) {
                 r = link_add(m, device, &link);
                 if (r < 0) {

-                        if (r == -EEXIST)
-                                log_debug("%s: link already exists, ignoring",
-                                          link->ifname);
-                        else
-                                log_error("%s: could not handle link: %s",
-                                          udev_device_get_sysname(device),
-                                          strerror(-r));
+                        log_error("%s: could not handle link: %s",
+                                  udev_device_get_sysname(device),
+                                  strerror(-r));

                 } else
                         log_debug("%s: link (with ifindex %" PRIu64") added",
                                   link->ifname, link->ifindex);
                 } else
                         log_debug("%s: link (with ifindex %" PRIu64") added",
                                   link->ifname, link->ifindex);


@@ -264,9 +265,12 @@ static int manager_rtnl_process_link(sd_rtnl *rtnl, sd_rtnl_message *message, vo
         Manager *m = userdata;
         Link *link;
         const char *name;
         Manager *m = userdata;
         Link *link;
         const char *name;

-        uint64_t ifindex_64;

         int r, ifindex;
 
         int r, ifindex;
 

+        assert(rtnl);
+        assert(message);
+        assert(m);
+

         r = sd_rtnl_message_link_get_ifindex(message, &ifindex);
         if (r < 0 || ifindex <= 0) {
                 log_debug("received RTM_NEWLINK message without valid ifindex");
         r = sd_rtnl_message_link_get_ifindex(message, &ifindex);
         if (r < 0 || ifindex <= 0) {
                 log_debug("received RTM_NEWLINK message without valid ifindex");


@@ -288,9 +292,8 @@ static int manager_rtnl_process_link(sd_rtnl *rtnl, sd_rtnl_message *message, vo
                 }
         }
 
                 }
         }
 

-        ifindex_64 = ifindex;
-        link = hashmap_get(m->links, &ifindex_64);
-        if (!link) {
+        r = link_get(m, ifindex, &link);
+        if (r < 0) {

                 log_debug("received RTM_NEWLINK message for untracked ifindex %d", ifindex);
                 return 0;
         }
                 log_debug("received RTM_NEWLINK message for untracked ifindex %d", ifindex);
                 return 0;
         }

diff --git a/src/network/networkd.h b/src/network/networkd.h
index 51d77570d40ebab12c8900b4bd68a04da98674fa..5a1bf1a4cac8aba722e462d7f0e54fadd02018d4 100644 (file)
--- a/src/network/networkd.h
+++ b/src/network/networkd.h
@@ -313,6 +313,7 @@ int config_parse_label(const char *unit, const char *filename, unsigned line,
 
 int link_new(Manager *manager, struct udev_device *device, Link **ret);
 void link_free(Link *link);
 
 int link_new(Manager *manager, struct udev_device *device, Link **ret);
 void link_free(Link *link);

+int link_get(Manager *m, int ifindex, Link **ret);

 int link_add(Manager *manager, struct udev_device *device, Link **ret);
 int link_configure(Link *link);
 
 int link_add(Manager *manager, struct udev_device *device, Link **ret);
 int link_configure(Link *link);