[master] Don't call pool_timer recusively

Add a flag to avoid supersede_lease calling pool_timer
recursively when pool_timer can't handle that.
rt38002

6 files changed, 61 insertions, 46 deletions

diff --git a/RELNOTES b/RELNOTES
index 052db310..70635b0b 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -197,6 +197,11 @@ by Eric Young (eay@cryptsoft.com).
   bug and supplying a patch.
   [ISC-Bugs #35712]
 
+- Avoid calling pool_timer() recursively from supersede_lease().  This could
+  result in leases changing state incorrectly or delaying the running of the
+  leae expiration code.
+  [ISC-Bugs #38002]
+
 			Changes since 4.3.1b1
 
 - Modify the linux and openwrt dhclient scripts to process information
diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index f5ac1f4c..b4468244 100644
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -3349,7 +3349,7 @@ void new_shared_network_interface (struct parse *,
 int subnet_inner_than(const struct subnet *, const struct subnet *, int);
 void enter_subnet (struct subnet *);
 void enter_lease (struct lease *);
-int supersede_lease (struct lease *, struct lease *, int, int, int);
+int supersede_lease (struct lease *, struct lease *, int, int, int, int);
 void make_binding_state_transition (struct lease *);
 int lease_copy (struct lease **, struct lease *, const char *, int);
 void release_lease (struct lease *, struct packet *);
diff --git a/server/dhcp.c b/server/dhcp.c
index b87b9535..7c5b7c77 100644
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -2963,7 +2963,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
 		 */
 		if ((use_old_lease == 0) &&
 		    !supersede_lease(lease, lt, commit,
-				     offer == DHCPACK, offer == DHCPACK)) {
+				     offer == DHCPACK, offer == DHCPACK, 0)) {
 #else /* defined(DELAYED_ACK) */
 		/*
 		 * If there already isn't a need for a lease commit, and we
@@ -2983,7 +2983,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
 		 */
 		if ((use_old_lease == 0) &&
 		    !supersede_lease(lease, lt, 0,
-				     !offer || offer == DHCPACK, 0)) {
+				     !offer || offer == DHCPACK, 0, 0)) {
 #endif
 			log_info ("%s: database update failed", msg);
 			free_lease_state (state, MDL);
diff --git a/server/failover.c b/server/failover.c
index e6d9fdd6..2486a735 100644
--- a/server/failover.c
+++ b/server/failover.c
@@ -2602,7 +2602,7 @@ dhcp_failover_pool_dobalance(dhcp_failover_state_t *state,
 			    lp->tstp = cur_time;
 			    lp->starts = cur_time;
 
-			    if (!supersede_lease(lp, NULL, 0, 1, 0) ||
+			    if (!supersede_lease(lp, NULL, 0, 1, 0, 0) ||
 			        !write_lease(lp))
 			    	    log_error("can't commit lease %s on "
 					      "giveaway", piaddr(lp->ip_addr));
@@ -5412,7 +5412,7 @@ isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state,
 	lease->rewind_binding_state = lt->next_binding_state;
 
 	/* Try to install the new information. */
-	if (!supersede_lease (lease, lt, 0, 0, 0) ||
+	if (!supersede_lease (lease, lt, 0, 0, 0, 0) ||
 	    !write_lease (lease)) {
 		message = "database update failed";
 	      bad:
@@ -5430,7 +5430,7 @@ isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state,
 		lease->tstp = cur_time;
 		lease->starts = cur_time;
 
-		if (!supersede_lease(lease, NULL, 0, 1, 0) ||
+		if (!supersede_lease(lease, NULL, 0, 1, 0, 0) ||
 		    !write_lease(lease))
 			log_error("can't commit lease %s for mac addr "
 				  "affinity", piaddr(lease->ip_addr));
@@ -5557,7 +5557,7 @@ isc_result_t dhcp_failover_process_bind_ack (dhcp_failover_state_t *state,
 		/* The peer will have made this state change, so set rewind. */
 		lease->rewind_binding_state = lease->next_binding_state;
 
-		supersede_lease(lease, (struct lease *)0, 0, 0, 0);
+		supersede_lease(lease, NULL, 0, 0, 0, 0);
 		write_lease(lease);
 
 		/* Lease has returned to FREE state from the
@@ -5582,8 +5582,7 @@ isc_result_t dhcp_failover_process_bind_ack (dhcp_failover_state_t *state,
 		if (lease->desired_binding_state != lease->binding_state) {
 			lease->next_binding_state =
 				lease->desired_binding_state;
-			supersede_lease(lease,
-					(struct lease *)0, 0, 0, 0);
+			supersede_lease(lease, NULL, 0, 0, 0, 0);
 		}
 		write_lease(lease);
 		/* Commit the lease only after a two-second timeout,
@@ -5613,7 +5612,7 @@ isc_result_t dhcp_failover_process_bind_ack (dhcp_failover_state_t *state,
 		lease->next_binding_state = FTS_BACKUP;
 		lease->tstp = lease->starts = cur_time;
 
-		if (!supersede_lease(lease, NULL, 0, 1, 0) ||
+		if (!supersede_lease(lease, NULL, 0, 1, 0, 0) ||
 		    !write_lease(lease))
 			log_error("can't commit lease %s for "
 				  "client affinity", piaddr(lease->ip_addr));
diff --git a/server/mdb.c b/server/mdb.c
index cf9420a1..425340be 100644
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -1091,11 +1091,12 @@ void enter_lease (lease)
    list of leases by expiry time so that we can always find the oldest
    lease. */
 
-int supersede_lease (comp, lease, commit, propogate, pimmediate)
+int supersede_lease (comp, lease, commit, propogate, pimmediate, from_pool)
 	struct lease *comp, *lease;
 	int commit;
 	int propogate;
 	int pimmediate;
+	int from_pool;
 {
 	struct lease *lp, **lq, *prev;
 	struct timeval tv;
@@ -1446,15 +1447,17 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate)
 		dhcp_failover_pool_check(comp->pool);
 #endif
 
-	/* If the current binding state has already expired, do an
-	   expiry event right now. */
+	/* If the current binding state has already expired and we haven't
+	 * been called from pool_timer, do an expiry event right now.
+	 */
 	/* XXX At some point we should optimize this so that we don't
 	   XXX write the lease twice, but this is a safe way to fix the
 	   XXX problem for 3.0 (I hope!). */
-	if ((commit || !pimmediate) &&
-	    comp -> sort_time < cur_time &&
-	    comp -> next_binding_state != comp -> binding_state)
-		pool_timer (comp -> pool);
+	if ((from_pool == 0) &&
+	    (commit || !pimmediate) &&
+	    (comp->sort_time < cur_time) &&
+	    (comp->next_binding_state != comp->binding_state))
+		pool_timer(comp->pool);
 
 	return 1;
 }
@@ -1779,7 +1782,7 @@ void release_lease (lease, packet)
 #else
 		lease -> next_binding_state = FTS_FREE;
 #endif
-		supersede_lease (lease, (struct lease *)0, 1, 1, 1);
+		supersede_lease(lease, NULL, 1, 1, 1, 0);
 	}
 }
 
@@ -1812,7 +1815,7 @@ void abandon_lease (lease, message)
 	lt -> uid = (unsigned char *)0;
 	lt -> uid_len = 0;
 	lt -> uid_max = 0;
-	supersede_lease (lease, lt, 1, 1, 1);
+	supersede_lease (lease, lt, 1, 1, 1, 0);
 	lease_dereference (&lt, MDL);
 }
 
@@ -1854,7 +1857,7 @@ void dissociate_lease (lease)
 	lt -> uid = (unsigned char *)0;
 	lt -> uid_len = 0;
 	lt -> uid_max = 0;
-	supersede_lease (lease, lt, 1, 1, 1);
+	supersede_lease (lease, lt, 1, 1, 1, 0);
 	lease_dereference (&lt, MDL);
 }
 #endif
@@ -1864,8 +1867,8 @@ void pool_timer (vpool)
 	void *vpool;
 {
 	struct pool *pool;
-	struct lease *next = (struct lease *)0;
-	struct lease *lease = (struct lease *)0;
+	struct lease *next = NULL;
+	struct lease *lease = NULL;
 #define FREE_LEASES 0
 #define ACTIVE_LEASES 1
 #define EXPIRED_LEASES 2
@@ -1879,11 +1882,11 @@ void pool_timer (vpool)
 
 	pool = (struct pool *)vpool;
 
-	lptr [FREE_LEASES] = &pool -> free;
-	lptr [ACTIVE_LEASES] = &pool -> active;
-	lptr [EXPIRED_LEASES] = &pool -> expired;
-	lptr [ABANDONED_LEASES] = &pool -> abandoned;
-	lptr [BACKUP_LEASES] = &pool -> backup;
+	lptr[FREE_LEASES] = &pool->free;
+	lptr[ACTIVE_LEASES] = &pool->active;
+	lptr[EXPIRED_LEASES] = &pool->expired;
+	lptr[ABANDONED_LEASES] = &pool->abandoned;
+	lptr[BACKUP_LEASES] = &pool->backup;
 	lptr[RESERVED_LEASES] = &pool->reserved;
 
 	for (i = FREE_LEASES; i <= RESERVED_LEASES; i++) {
@@ -1919,20 +1922,20 @@ void pool_timer (vpool)
 				continue;
 		}
 #endif		
-		lease_reference (&lease, *(lptr [i]), MDL);
+		lease_reference(&lease, *(lptr [i]), MDL);
 
 		while (lease) {
 			/* Remember the next lease in the list. */
 			if (next)
-				lease_dereference (&next, MDL);
+				lease_dereference(&next, MDL);
 			if (lease -> next)
-				lease_reference (&next, lease -> next, MDL);
+				lease_reference(&next, lease->next, MDL);
 
 			/* If we've run out of things to expire on this list,
 			   stop. */
-			if (lease -> sort_time > cur_time) {
-				if (lease -> sort_time < next_expiry)
-					next_expiry = lease -> sort_time;
+			if (lease->sort_time > cur_time) {
+				if (lease->sort_time < next_expiry)
+					next_expiry = lease->sort_time;
 				break;
 			}
 
@@ -1962,27 +1965,35 @@ void pool_timer (vpool)
 					lease->next_binding_state =
 						   lease->rewind_binding_state;
 #endif
-				supersede_lease(lease, NULL, 1, 1, 1);
+				supersede_lease(lease, NULL, 1, 1, 1, 1);
 			}
 
-			lease_dereference (&lease, MDL);
+			lease_dereference(&lease, MDL);
 			if (next)
-				lease_reference (&lease, next, MDL);
+				lease_reference(&lease, next, MDL);
 		}
 		if (next)
-			lease_dereference (&next, MDL);
+			lease_dereference(&next, MDL);
 		if (lease)
-			lease_dereference (&lease, MDL);
+			lease_dereference(&lease, MDL);
 	}
-	if (next_expiry != MAX_TIME) {
-		pool -> next_event_time = next_expiry;
-		tv . tv_sec = pool -> next_event_time;
-		tv . tv_usec = 0;
+
+	/* If we found something to expire and its expiration time
+	 * is either less than the current expiration time or the
+	 * current expiration time is already expired update the
+	 * timer.
+	 */
+	if ((next_expiry != MAX_TIME) &&
+	    ((pool->next_event_time > next_expiry) ||
+	     (pool->next_event_time <= cur_time))) {
+		pool->next_event_time = next_expiry;
+		tv.tv_sec = pool->next_event_time;
+		tv.tv_usec = 0;
 		add_timeout (&tv, pool_timer, pool,
 			     (tvref_t)pool_reference,
 			     (tvunref_t)pool_dereference);
 	} else
-		pool -> next_event_time = MIN_TIME;
+		pool->next_event_time = MIN_TIME;
 
 }
 
diff --git a/server/omapi.c b/server/omapi.c
index 154f8327..17292e23 100644
--- a/server/omapi.c
+++ b/server/omapi.c
@@ -227,7 +227,7 @@ isc_result_t dhcp_lease_set_value  (omapi_object_t *h,
 
 	    if (lease -> binding_state != bar) {
 		lease -> next_binding_state = bar;
-		if (supersede_lease (lease, 0, 1, 1, 1)) {
+		if (supersede_lease (lease, NULL, 1, 1, 1, 0)) {
 			log_info ("lease %s state changed from %s to %s",
 				  piaddr(lease->ip_addr), ols, nls);
 			return ISC_R_SUCCESS;
@@ -260,7 +260,7 @@ isc_result_t dhcp_lease_set_value  (omapi_object_t *h,
 		return status;
 	    old_lease_end = lease->ends;
 	    lease->ends = lease_end;
-	    if (supersede_lease (lease, 0, 1, 1, 1)) {
+	    if (supersede_lease (lease, NULL, 1, 1, 1, 0)) {
 		log_info ("lease %s end changed from %lu to %lu",
 			  piaddr(lease->ip_addr), old_lease_end, lease_end);
 		return ISC_R_SUCCESS;
@@ -279,7 +279,7 @@ isc_result_t dhcp_lease_set_value  (omapi_object_t *h,
 			   (lease->flags & ~EPHEMERAL_FLAGS);
 	    if(oldflags == lease->flags)
 		return ISC_R_SUCCESS;
-	    if (!supersede_lease(lease, NULL, 1, 1, 1)) {
+	    if (!supersede_lease(lease, NULL, 1, 1, 1, 0)) {
 		log_error("Failed to update flags for lease %s.",
 			  piaddr(lease->ip_addr));
 		return ISC_R_IOERROR;