[v4_2] 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

diff --git a/RELNOTES b/RELNOTES
index cba080509a196b91a1378d617e9e3a9545136c7f..413630bcacff104eeb8da506f5aa21a8ded3ba55 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -158,12 +158,18 @@ by Eric Young (eay@cryptsoft.com).
   will occur whether or not the server is compiled for failover.
   [ISC-Bugs #36960]
 
+- 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.2.7rc1
 
 - None
 
                        Changes since 4.2.7b1
 
+
 - Modify the linux and openwrt dhclient scripts to process information
   from a stateless request.  Thanks to Jiri Popelka at Red Hat for the
   bug report and patch.

diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index 59328d722fb0019fe9ea8d9b020a4434e293328c..2b1543062d30d570b962fa75cd00e9975082ad2a 100644 (file)
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -3265,7 +3265,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 242beaa978208154fcceb953f1a97d0cce53b60a..a724fb94605785e3ff418e4b6a198956e5d1a223 100644 (file)
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -2509,7 +2509,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
                 * a conflict.
                 */
                if (!supersede_lease(lease, lt, !offer || (offer == DHCPACK),
-                                    offer == DHCPACK, offer == DHCPACK)) {
+                                    offer == DHCPACK, offer == DHCPACK, 0)) {
 #else /* defined(DELAYED_ACK) */
                /* Install the new information on 'lt' onto the lease at
                 * 'lease'.  We will not 'commit' this information to disk
@@ -2520,8 +2520,9 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
                 * BOOTREPLY either); we may give the same lease out to a
                 * different client, and that would be a conflict.
                 */
+
                if (!supersede_lease(lease, lt, 0, !offer || offer == DHCPACK,
-                                    0)) {
+                                    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 6eae368744fcd00b33a529d235e0599ec4b7d502..5018b8a7a05ccedf8f27b42765b2fb97c86c26e5 100644 (file)
--- a/server/failover.c
+++ b/server/failover.c
@@ -2594,7 +2594,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));
@@ -5404,7 +5404,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:
@@ -5422,7 +5422,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));
@@ -5549,7 +5549,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
@@ -5574,8 +5574,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,
@@ -5605,7 +5604,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 2eae892191271730d3300440e52483d0cfd25ad9..5503ea02933729f574648b7306d63ccce9369714 100644 (file)
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -1044,11 +1044,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;
@@ -1398,15 +1399,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;
 }
@@ -1735,7 +1738,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);
        }
 }
 
@@ -1768,7 +1771,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);
 }
 
@@ -1810,7 +1813,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
@@ -1820,8 +1823,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
@@ -1835,11 +1838,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++) {
@@ -1875,20 +1878,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;
                        }
 
@@ -1918,27 +1921,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 e46b00da0ab875aab2d6b1ca2f2ecfef0fd6991a..36e70a89461ee53a0ac4972ccd7760e31ef53956 100644 (file)
--- 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;