From: Razvan Becheriu <razvan@isc.org>
Date: Fri, 14 Mar 2025 08:20:06 +0000 (+0200)
Subject: [#3798] drop packet on db race
X-Git-Tag: Kea-2.6.2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cd63721cd633861051714450c3bab08735018b97;p=thirdparty%2Fkea.git

[#3798] drop packet on db race
---

diff --git a/ChangeLog b/ChangeLog
index f749df6d36..27d38147ca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2260.	[bug]		razvan
+	Fixed a bug which was causing the allocation engine to reject the
+	lease if a data race caused by a different server updating the
+	shared database entries was detected. The entire packet is now
+	dropped in this particular case. This applies to both kea-dhp4
+	and kea-dhcp6 servers.
+	(Gitlab #3798, #3648)
+
 2259.	[bug]		marcin
 	Prevent the clients from declining expired or released leases.
 	Only a valid lease assigned to the declining client can now
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 6019012b1f..c7c84b1edb 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -15,6 +15,7 @@
 #include <dhcpsrv/alloc_engine.h>
 #include <dhcpsrv/alloc_engine_log.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/dhcpsrv_exceptions.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/host_mgr.h>
 #include <dhcpsrv/host.h>
@@ -628,6 +629,9 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
             return (leases);
         }
 
+    } catch (const NoSuchLease& e) {
+        isc_throw(NoSuchLease, "detected data race in AllocEngine::allocateLeases6: " << e.what());
+
     } catch (const isc::Exception& e) {
 
         // Some other error, return an empty lease.
@@ -2172,6 +2176,9 @@ AllocEngine::renewLeases6(ClientContext6& ctx) {
 
         return (leases);
 
+    } catch (const NoSuchLease& e) {
+        isc_throw(NoSuchLease, "detected data race in AllocEngine::renewLeases6: " << e.what());
+
     } catch (const isc::Exception& e) {
 
         // Some other error, return an empty lease.
@@ -3704,6 +3711,9 @@ AllocEngine::allocateLease4(ClientContext4& ctx) {
             ctx.new_lease_ = requestLease4(ctx);
         }
 
+    } catch (const NoSuchLease& e) {
+        isc_throw(NoSuchLease, "detected data race in AllocEngine::allocateLease4: " << e.what());
+
     } catch (const isc::Exception& e) {
         // Some other error, return an empty lease.
         LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_V4_ALLOC_ERROR)
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index db4f5d577c..350a0397ad 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -1831,8 +1831,9 @@ Memfile_LeaseMgr::updateLease4Internal(const Lease4Ptr& lease) {
         ((*lease_it)->valid_lft_ != lease->current_valid_lft_))) {
         // For test purpose only: check that the lease has not changed in
         // the database.
-        isc_throw(NoSuchLease, "failed to update the lease with address "
-                  << lease->addr_ << " - lease has changed in database");
+        isc_throw(NoSuchLease, "unable to update lease for address " <<
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Try to write a lease to disk first. If this fails, the lease will
@@ -1894,8 +1895,9 @@ Memfile_LeaseMgr::updateLease6Internal(const Lease6Ptr& lease) {
         ((*lease_it)->valid_lft_ != lease->current_valid_lft_))) {
         // For test purpose only: check that the lease has not changed in
         // the database.
-        isc_throw(NoSuchLease, "failed to update the lease with address "
-                  << lease->addr_ << " - lease has changed in database");
+        isc_throw(NoSuchLease, "unable to update lease for address " <<
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Try to write a lease to disk first. If this fails, the lease will
diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index 0f4fc005e6..3bb5d1d83c 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -3265,7 +3265,8 @@ MySqlLeaseMgr::updateLeaseCommon(MySqlLeaseContextPtr& ctx,
     // If no rows affected, lease doesn't exist.
     if (affected_rows == 0) {
         isc_throw(NoSuchLease, "unable to update lease for address " <<
-                  lease->addr_.toText() << " as it does not exist");
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Should not happen - primary key constraint should only have selected
diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
index 7bb7bf30e4..e88c1d25cf 100644
--- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
@@ -2499,7 +2499,8 @@ PgSqlLeaseMgr::updateLeaseCommon(PgSqlLeaseContextPtr& ctx,
     // If no rows affected, lease doesn't exist.
     if (affected_rows == 0) {
         isc_throw(NoSuchLease, "unable to update lease for address " <<
-                  lease->addr_.toText() << " as it does not exist");
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Should not happen - primary key constraint should only have selected