Merge pull request #25142 from yuwata/network-fix-race-in-device-renaming-vs-dhcp

network,dhcp: fix theoretical race in device renaming

src/libsystemd-network/dhcp-identifier.c

@@ -4,15 +4,11 @@
 #include <net/ethernet.h>
 #include <net/if_arp.h>
 
-#include "sd-device.h"
-#include "sd-id128.h"
-
 #include "dhcp-identifier.h"
 #include "netif-util.h"
 #include "siphash24.h"
 #include "sparse-endian.h"
 #include "string-table.h"
-#include "udev-util.h"
 
 #define HASH_KEY       SD_ID128_MAKE(80,11,8c,c2,fe,4a,03,ee,3e,d6,0c,6f,36,39,14,09)
 #define APPLICATION_ID SD_ID128_MAKE(a5,0a,d1,12,bf,60,45,77,a2,fb,74,1a,b1,95,5b,03)
@@ -207,48 +203,20 @@ int dhcp_identifier_set_duid(
 }
 
 int dhcp_identifier_set_iaid(
-                int ifindex,
+                sd_device *dev,
                 const struct hw_addr_data *hw_addr,
                 bool legacy_unstable_byteorder,
-                bool use_mac,
                 void *ret) {
 
-        /* name is a pointer to memory in the sd_device struct, so must
-         * have the same scope */
-        _cleanup_(sd_device_unrefp) sd_device *device = NULL;
         const char *name = NULL;
         uint32_t id32;
         uint64_t id;
-        int r;
 
-        assert(ifindex > 0);
         assert(hw_addr);
         assert(ret);
 
-        if (udev_available() && !use_mac) {
+        if (dev)
-                /* udev should be around */
+                name = net_get_persistent_name(dev);
-
-                r = sd_device_new_from_ifindex(&device, ifindex);
-                if (r < 0)
-                        return r;
-
-                r = sd_device_get_is_initialized(device);
-                if (r < 0)
-                        return r;
-                if (r == 0)
-                        /* not yet ready */
-                        return -EBUSY;
-
-                r = device_is_renaming(device);
-                if (r < 0)
-                        return r;
-                if (r > 0)
-                        /* device is under renaming */
-                        return -EBUSY;
-
-                name = net_get_persistent_name(device);
-        }
-
         if (name)
                 id = siphash24(name, strlen(name), HASH_KEY.bytes);
         else

src/libsystemd-network/dhcp-identifier.h

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include "sd-device.h"
 #include "sd-id128.h"
 
 #include "ether-addr-util.h"
@@ -66,10 +67,9 @@ int dhcp_identifier_set_duid(
                 struct duid *ret_duid,
                 size_t *ret_len);
 int dhcp_identifier_set_iaid(
-                int ifindex,
+                sd_device *dev,
                 const struct hw_addr_data *hw_addr,
                 bool legacy_unstable_byteorder,
-                bool use_mac,
                 void *ret);
 
 const char *duid_type_to_string(DUIDType t) _const_;

src/libsystemd-network/dhcp6-internal.h

@@ -48,6 +48,8 @@ struct sd_dhcp6_client {
         int event_priority;
         int fd;
 
+        sd_device *dev;
+
         DHCP6State state;
         bool information_request;
         usec_t information_request_time_usec;
@@ -78,7 +80,7 @@ struct sd_dhcp6_client {
         sd_dhcp6_client_callback_t callback;
         void *userdata;
 
-        /* Ignore ifindex when generating iaid. See dhcp_identifier_set_iaid(). */
+        /* Ignore machine-ID when generating DUID. See dhcp_identifier_set_duid_en(). */
         bool test_mode;
 };
 

src/libsystemd-network/sd-dhcp-client.c

@@ -14,6 +14,7 @@
 #include "sd-dhcp-client.h"
 
 #include "alloc-util.h"
+#include "device-util.h"
 #include "dhcp-identifier.h"
 #include "dhcp-internal.h"
 #include "dhcp-lease-internal.h"
@@ -77,8 +78,12 @@ struct sd_dhcp_client {
         sd_event *event;
         int event_priority;
         sd_event_source *timeout_resend;
+
         int ifindex;
         char *ifname;
+
+        sd_device *dev;
+
         int fd;
         uint16_t port;
         union sockaddr_union link;
@@ -117,7 +122,7 @@ struct sd_dhcp_client {
         usec_t start_delay;
         int ip_service_type;
 
-        /* Ignore ifindex when generating iaid. See dhcp_identifier_set_iaid(). */
+        /* Ignore machine-ID when generating DUID. See dhcp_identifier_set_duid_en(). */
         bool test_mode;
 };
 
@@ -420,9 +425,8 @@ static int dhcp_client_set_iaid_duid_internal(
                 if (iaid_set)
                         client->client_id.ns.iaid = htobe32(iaid);
                 else {
-                        r = dhcp_identifier_set_iaid(client->ifindex, &client->hw_addr,
+                        r = dhcp_identifier_set_iaid(client->dev, &client->hw_addr,
                                                      /* legacy_unstable_byteorder = */ true,
-                                                     /* use_mac = */ client->test_mode,
                                                      &client->client_id.ns.iaid);
                         if (r < 0)
                                 return log_dhcp_client_errno(client, r, "Failed to set IAID: %m");
@@ -799,9 +803,8 @@ static int client_message_init(
 
                 client->client_id.type = 255;
 
-                r = dhcp_identifier_set_iaid(client->ifindex, &client->hw_addr,
+                r = dhcp_identifier_set_iaid(client->dev, &client->hw_addr,
                                              /* legacy_unstable_byteorder = */ true,
-                                             /* use_mac = */ client->test_mode,
                                              &client->client_id.ns.iaid);
                 if (r < 0)
                         return r;
@@ -2153,6 +2156,12 @@ sd_event *sd_dhcp_client_get_event(sd_dhcp_client *client) {
         return client->event;
 }
 
+int sd_dhcp_client_attach_device(sd_dhcp_client *client, sd_device *dev) {
+        assert_return(client, -EINVAL);
+
+        return device_unref_and_replace(client->dev, dev);
+}
+
 static sd_dhcp_client *dhcp_client_free(sd_dhcp_client *client) {
         if (!client)
                 return NULL;
@@ -2168,6 +2177,8 @@ static sd_dhcp_client *dhcp_client_free(sd_dhcp_client *client) {
 
         sd_dhcp_client_detach_event(client);
 
+        sd_device_unref(client->dev);
+
         set_free(client->req_opts);
         free(client->hostname);
         free(client->vendor_class_identifier);

src/libsystemd-network/sd-dhcp6-client.c

@@ -11,6 +11,7 @@
 #include "sd-dhcp6-client.h"
 
 #include "alloc-util.h"
+#include "device-util.h"
 #include "dhcp-identifier.h"
 #include "dhcp6-internal.h"
 #include "dhcp6-lease-internal.h"
@@ -299,9 +300,8 @@ static int client_ensure_iaid(sd_dhcp6_client *client) {
         if (client->iaid_set)
                 return 0;
 
-        r = dhcp_identifier_set_iaid(client->ifindex, &client->hw_addr,
+        r = dhcp_identifier_set_iaid(client->dev, &client->hw_addr,
                                      /* legacy_unstable_byteorder = */ true,
-                                     /* use_mac = */ client->test_mode,
                                      &iaid);
         if (r < 0)
                 return r;
@@ -1446,6 +1446,12 @@ sd_event *sd_dhcp6_client_get_event(sd_dhcp6_client *client) {
         return client->event;
 }
 
+int sd_dhcp6_client_attach_device(sd_dhcp6_client *client, sd_device *dev) {
+        assert_return(client, -EINVAL);
+
+        return device_unref_and_replace(client->dev, dev);
+}
+
 static sd_dhcp6_client *dhcp6_client_free(sd_dhcp6_client *client) {
         if (!client)
                 return NULL;
@@ -1461,6 +1467,8 @@ static sd_dhcp6_client *dhcp6_client_free(sd_dhcp6_client *client) {
 
         client->fd = safe_close(client->fd);
 
+        sd_device_unref(client->dev);
+
         free(client->req_opts);
         free(client->fqdn);
         free(client->mudurl);

src/libsystemd-network/test-dhcp-client.c

@@ -144,10 +144,8 @@ static void test_dhcp_identifier_set_iaid(void) {
         uint32_t iaid_legacy;
         be32_t iaid;
 
-        assert_se(dhcp_identifier_set_iaid(42, &hw_addr, /* legacy = */ true,
+        assert_se(dhcp_identifier_set_iaid(NULL, &hw_addr, /* legacy = */ true, &iaid_legacy) >= 0);
-                                           /* use_mac = */ true, &iaid_legacy) >= 0);
+        assert_se(dhcp_identifier_set_iaid(NULL, &hw_addr, /* legacy = */ false, &iaid) >= 0);
-        assert_se(dhcp_identifier_set_iaid(42, &hw_addr, /* legacy = */ false,
-                                           /* use_mac = */ true, &iaid) >= 0);
 
         /* we expect, that the MAC address was hashed. The legacy value is in native
          * endianness. */
@@ -169,7 +167,7 @@ static int check_options(uint8_t code, uint8_t len, const void *option, void *us
                 size_t duid_len;
 
                 assert_se(dhcp_identifier_set_duid_en(/* test_mode = */ true, &duid, &duid_len) >= 0);
-                assert_se(dhcp_identifier_set_iaid(42, &hw_addr, /* legacy = */ true, /* use_mac = */ true, &iaid) >= 0);
+                assert_se(dhcp_identifier_set_iaid(NULL, &hw_addr, /* legacy = */ true, &iaid) >= 0);
 
                 assert_se(len == sizeof(uint8_t) + sizeof(uint32_t) + duid_len);
                 assert_se(len == 19);

src/network/networkd-dhcp4.c

@@ -1357,6 +1357,10 @@ static int dhcp4_configure(Link *link) {
         if (r < 0)
                 return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to attach event to DHCPv4 client: %m");
 
+        r = sd_dhcp_client_attach_device(link->dhcp_client, link->dev);
+        if (r < 0)
+                return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to attach device: %m");
+
         r = sd_dhcp_client_set_mac(link->dhcp_client,
                                    link->hw_addr.bytes,
                                    link->bcast_addr.length > 0 ? link->bcast_addr.bytes : NULL,

src/network/networkd-dhcp6.c

@@ -594,6 +594,10 @@ static int dhcp6_configure(Link *link) {
         if (r < 0)
                 return log_link_debug_errno(link, r, "DHCPv6 CLIENT: Failed to attach event: %m");
 
+        r = sd_dhcp6_client_attach_device(client, link->dev);
+        if (r < 0)
+                return log_link_debug_errno(link, r, "DHCPv6 CLIENT: Failed to attach device: %m");
+
         r = dhcp6_set_identifier(link, client);
         if (r < 0)
                 return log_link_debug_errno(link, r, "DHCPv6 CLIENT: Failed to set identifier: %m");

src/network/networkd-link.c

@@ -1410,6 +1410,18 @@ static int link_initialized(Link *link, sd_device *device) {
          * or sysattrs) may be outdated. */
         device_unref_and_replace(link->dev, device);
 
+        if (link->dhcp_client) {
+                r = sd_dhcp_client_attach_device(link->dhcp_client, link->dev);
+                if (r < 0)
+                        log_link_warning_errno(link, r, "Failed to attach device to DHCPv4 client, ignoring: %m");
+        }
+
+        if (link->dhcp6_client) {
+                r = sd_dhcp6_client_attach_device(link->dhcp6_client, link->dev);
+                if (r < 0)
+                        log_link_warning_errno(link, r, "Failed to attach device to DHCPv6 client, ignoring: %m");
+        }
+
         r = link_set_sr_iov_ifindices(link);
         if (r < 0)
                 log_link_warning_errno(link, r, "Failed to manage SR-IOV PF and VF ports, ignoring: %m");

src/systemd/sd-dhcp-client.h

@@ -25,6 +25,7 @@
 #include <sys/types.h>
 #include <stdbool.h>
 
+#include "sd-device.h"
 #include "sd-dhcp-lease.h"
 #include "sd-dhcp-option.h"
 #include "sd-event.h"
@@ -337,6 +338,7 @@ int sd_dhcp_client_attach_event(
                 int64_t priority);
 int sd_dhcp_client_detach_event(sd_dhcp_client *client);
 sd_event *sd_dhcp_client_get_event(sd_dhcp_client *client);
+int sd_dhcp_client_attach_device(sd_dhcp_client *client, sd_device *dev);
 
 _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_dhcp_client, sd_dhcp_client_unref);
 

src/systemd/sd-dhcp6-client.h

@@ -23,6 +23,7 @@
 #include <net/ethernet.h>
 #include <sys/types.h>
 
+#include "sd-device.h"
 #include "sd-dhcp6-lease.h"
 #include "sd-dhcp6-option.h"
 #include "sd-event.h"
@@ -279,6 +280,7 @@ int sd_dhcp6_client_attach_event(
                 int64_t priority);
 int sd_dhcp6_client_detach_event(sd_dhcp6_client *client);
 sd_event *sd_dhcp6_client_get_event(sd_dhcp6_client *client);
+int sd_dhcp6_client_attach_device(sd_dhcp6_client *client, sd_device *dev);
 sd_dhcp6_client *sd_dhcp6_client_ref(sd_dhcp6_client *client);
 sd_dhcp6_client *sd_dhcp6_client_unref(sd_dhcp6_client *client);
 int sd_dhcp6_client_new(sd_dhcp6_client **ret);