From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Wed, 26 Jan 2022 15:17:23 +0000 (+0900)
Subject: sd-dhcp-server: set DHCPLease::server before hashmap_put()
X-Git-Tag: v251-rc1~396^2~13
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8a7d048d1ddb8916482f1422405d3e0e4bccb279;p=thirdparty%2Fsystemd.git

sd-dhcp-server: set DHCPLease::server before hashmap_put()

Otherwise, if the second push is failed, then the first hashmap contains
dirty entry.

Also, this makes hashmap_remove_value() used when removing leases to
make not wrong lease is removed from the hashmap.

Note, this just hide the root cause of the issue #22253, which will be
fixed in later commit.

Fixes #22253.
---

diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c
index 68a7916f641..921dad5c05e 100644
--- a/src/libsystemd-network/sd-dhcp-server.c
+++ b/src/libsystemd-network/sd-dhcp-server.c
@@ -30,19 +30,10 @@ static DHCPLease *dhcp_lease_free(DHCPLease *lease) {
                 return NULL;
 
         if (lease->server) {
-                DHCPLease *e;
-
-                e = hashmap_get(lease->server->bound_leases_by_client_id, &lease->client_id);
-                if (e == lease) {
-                        hashmap_remove(lease->server->bound_leases_by_address, UINT32_TO_PTR(lease->address));
-                        hashmap_remove(lease->server->bound_leases_by_client_id, &lease->client_id);
-                }
-
-                e = hashmap_get(lease->server->static_leases_by_client_id, &lease->client_id);
-                if (e == lease) {
-                        hashmap_remove(lease->server->static_leases_by_address, UINT32_TO_PTR(lease->address));
-                        hashmap_remove(lease->server->static_leases_by_client_id, &lease->client_id);
-                }
+                hashmap_remove_value(lease->server->bound_leases_by_address, UINT32_TO_PTR(lease->address), lease);
+                hashmap_remove_value(lease->server->bound_leases_by_client_id, &lease->client_id, lease);
+                hashmap_remove_value(lease->server->static_leases_by_address, UINT32_TO_PTR(lease->address), lease);
+                hashmap_remove_value(lease->server->static_leases_by_client_id, &lease->client_id, lease);
         }
 
         free(lease->client_id.data);
@@ -1118,7 +1109,10 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
 
                         log_dhcp_server(server, "ACK (0x%x)", be32toh(req->message->xid));
 
-                        dhcp_lease_free(hashmap_remove(server->bound_leases_by_client_id, &lease->client_id));
+                        dhcp_lease_free(hashmap_get(server->bound_leases_by_client_id, &lease->client_id));
+
+                        lease->server = server; /* This must be set just before hashmap_put(). */
+
                         r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
                         if (r < 0)
                                 return log_dhcp_server_errno(server, r, "Could not save lease: %m");
@@ -1127,7 +1121,6 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                         if (r < 0)
                                 return log_dhcp_server_errno(server, r, "Could not save lease: %m");
 
-                        lease->server = server;
                         TAKE_PTR(lease);
 
                         if (server->callback)
@@ -1168,6 +1161,8 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
 
                         log_dhcp_server(server, "ACK (0x%x)", be32toh(req->message->xid));
 
+                        lease->server = server; /* This must be set just before hashmap_put(). */
+
                         r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
                         if (r < 0)
                                 return log_dhcp_server_errno(server, r, "Could not save lease: %m");
@@ -1175,7 +1170,6 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                         if (r < 0)
                                 return log_dhcp_server_errno(server, r, "Could not save lease: %m");
 
-                        lease->server = server;
                         TAKE_PTR(new_lease);
 
                         if (server->callback)
@@ -1599,13 +1593,10 @@ int sd_dhcp_server_set_static_lease(
                         .data = client_id,
                 };
 
-                dhcp_lease_free(hashmap_remove(server->static_leases_by_client_id, &c));
+                dhcp_lease_free(hashmap_get(server->static_leases_by_client_id, &c));
                 return 0;
         }
 
-        if (hashmap_contains(server->static_leases_by_address, UINT32_TO_PTR(address->s_addr)))
-                return -EEXIST;
-
         lease = new(DHCPLease, 1);
         if (!lease)
                 return -ENOMEM;
@@ -1618,6 +1609,8 @@ int sd_dhcp_server_set_static_lease(
         if (!lease->client_id.data)
                 return -ENOMEM;
 
+        lease->server = server; /* This must be set just before hashmap_put(). */
+
         r = hashmap_ensure_put(&server->static_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
         if (r < 0)
                 return r;
@@ -1625,7 +1618,6 @@ int sd_dhcp_server_set_static_lease(
         if (r < 0)
                 return r;
 
-        lease->server = server;
         TAKE_PTR(lease);
         return 0;
 }