[master] Fixed server crash after billing class is deleted

    Merges in rt39978.

diff --git a/RELNOTES b/RELNOTES
index f4e2c41b7dcabe4eeb33c7103ce16802ba92bad6..e670384a0ffd9fe35ed688d0e14bb748aed3533f 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -174,6 +174,11 @@ by Eric Young (eay@cryptsoft.com).
   within the same subnet and many people configure them in that fashion.
   [ISC-Bugs #40077]
 
+- Fixed a server crash that could occur when the server attempts to remove
+  the billing class from the last lease billed to a dynamic class after said
+  class has been deleted.
+  [ISC-Bugs #39978]
+
                        Changes since 4.3.2rc2
 - None
 

diff --git a/client/dhclient.c b/client/dhclient.c
index d14a9cf7880f6afd131b2bf68d2377a1ce7ecc34..be2af6ad657bd517ec0466545b0f0a34503f3cfb 100644 (file)
--- a/client/dhclient.c
+++ b/client/dhclient.c
@@ -870,11 +870,9 @@ void classify (packet, class)
 {
 }
 
-int unbill_class (lease, class)
+void unbill_class (lease)
        struct lease *lease;
-       struct class *class;
 {
-       return 0;
 }
 
 int find_subnet (struct subnet **sp,

diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index 4e75421e72098f1af70f47d22cc0ec3515d732fd..20bf30e948557412668acab47fd47c257bbb135a 100644 (file)
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -3127,7 +3127,7 @@ void classify (struct packet *, struct class *);
 isc_result_t unlink_class (struct class **class);
 isc_result_t find_class (struct class **, const char *,
                         const char *, int);
-int unbill_class (struct lease *, struct class *);
+void unbill_class (struct lease *);
 int bill_class (struct lease *, struct class *);
 
 /* execute.c */

diff --git a/server/class.c b/server/class.c
index 48d4a6cb3d09736acd0f13a4435df527c7ff779b..5c1d8e73b6c581db0285b04741724e4603c69e7b 100644 (file)
--- a/server/class.c
+++ b/server/class.c
@@ -3,7 +3,7 @@
    Handling for client classes. */
 
 /*
- * Copyright (c) 2009,2012-2014 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012-2015 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 2004,2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1998-2003 by Internet Software Consortium
  *
@@ -235,7 +235,7 @@ isc_result_t unlink_class(struct class **class) {
        return ISC_R_NOTFOUND;
 }
 
-       
+
 isc_result_t find_class (struct class **class, const char *name,
                         const char *file, int line)
 {
@@ -251,24 +251,53 @@ isc_result_t find_class (struct class **class, const char *name,
        return ISC_R_NOTFOUND;
 }
 
-int unbill_class (lease, class)
+/* Removes the billing class from a lease
+ *
+ * Note that because classes can be created and removed dynamically, it is
+ * possible that the class to which a lease was billed has since been deleted.
+ * To cover the case where the lease is the last reference to a deleted class
+ * we remove the lease reference from the class first, then the class from the
+ * lease.  To protect ourselves from the reverse situation, where the class is
+ * the last reference to the lease (unlikely), we create a guard reference to
+ * the lease, then remove it at the end.
+ */
+void unbill_class (lease)
        struct lease *lease;
-       struct class *class;
 {
        int i;
+       struct class* class = lease->billing_class;
+       struct lease* refholder = NULL;
 
-       for (i = 0; i < class -> lease_limit; i++)
-               if (class -> billed_leases [i] == lease)
+       /* if there's no billing to remove, nothing to do */
+       if (class == NULL) {
+               return;
+       }
+
+       /* Find the lease in the list of the class's billed leases */
+       for (i = 0; i < class->lease_limit; i++) {
+               if (class->billed_leases[i] == lease)
                        break;
-       if (i == class -> lease_limit) {
+       }
+
+       /* Create guard reference, so class cannot be last reference to lease */
+       lease_reference(&refholder, lease, MDL);
+
+       /* If the class doesn't have the lease, then something is broken
+        * programmatically.  We'll log it but skip the lease dereference. */
+       if (i == class->lease_limit) {
                log_error ("lease %s unbilled with no billing arrangement.",
-                     piaddr (lease -> ip_addr));
-               return 0;
+                          piaddr(lease->ip_addr));
+       } else {
+               /* Remove the lease from the class */
+               lease_dereference(&class->billed_leases[i], MDL);
+               class->leases_consumed--;
        }
-       class_dereference (&lease -> billing_class, MDL);
-       lease_dereference (&class -> billed_leases [i], MDL);
-       class -> leases_consumed--;
-       return 1;
+
+       /* Remove the class from the lease */
+       class_dereference(&lease->billing_class, MDL);
+
+       /* Ditch our guard reference */
+       lease_dereference(&refholder, MDL);
 }
 
 int bill_class (lease, class)
@@ -279,7 +308,7 @@ int bill_class (lease, class)
 
        if (lease -> billing_class) {
                log_error ("lease billed with existing billing arrangement.");
-               unbill_class (lease, lease -> billing_class);
+               unbill_class (lease);
        }
 
        if (class -> leases_consumed == class -> lease_limit)

diff --git a/server/dhcp.c b/server/dhcp.c
index 5db75f7551374c2156ac255e482ef6963d65feb9..1f007e2946be72ef0bd091f7e1fd0268c6b91b6b 100644 (file)
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -2321,7 +2321,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
                                    lease -> billing_class)
                                        break;
                        if (i == packet -> class_count) {
-                               unbill_class (lease, lease -> billing_class);
+                               unbill_class(lease);
                                /* Active lease billing change negates reuse */
                                if (lease->binding_state == FTS_ACTIVE) {
                                        lease->cannot_reuse = 1;
@@ -2373,7 +2373,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
                        if (offer == DHCPOFFER &&
                            lease->billing_class != NULL &&
                            lease->binding_state != FTS_ACTIVE)
-                               unbill_class(lease, lease->billing_class);
+                               unbill_class(lease);
 
                        /* Lease billing change negates reuse */
                        if (lease->billing_class != NULL) {

diff --git a/server/mdb.c b/server/mdb.c
index 6a637b5ef4e5d30b0af6134639548cc78ac8a4eb..219db506f7ec98595b71c3f63d6467be8dae1bd2 100644 (file)
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -1185,8 +1185,8 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate, from_pool)
 
        /* If the lease has been billed to a class, remove the billing. */
        if (comp -> billing_class != lease -> billing_class) {
-               if (comp -> billing_class)
-                       unbill_class (comp, comp -> billing_class);
+               if (comp->billing_class)
+                       unbill_class(comp);
                if (lease -> billing_class)
                        bill_class (comp, lease -> billing_class);
        }
@@ -1502,8 +1502,8 @@ void make_binding_state_transition (struct lease *lease)
                                (&lease->on_star.on_release, MDL);
                /* Get rid of client-specific bindings that are only
                   correct when the lease is active. */
-               if (lease -> billing_class)
-                       unbill_class (lease, lease -> billing_class);
+               if (lease->billing_class)
+                       unbill_class(lease);
                if (lease -> agent_options)
                        option_chain_head_dereference (&lease -> agent_options,
                                                       MDL);
@@ -1566,8 +1566,8 @@ void make_binding_state_transition (struct lease *lease)
 
                /* Get rid of client-specific bindings that are only
                   correct when the lease is active. */
-               if (lease -> billing_class)
-                       unbill_class (lease, lease -> billing_class);
+               if (lease->billing_class)
+                       unbill_class(lease);
                if (lease -> agent_options)
                        option_chain_head_dereference (&lease -> agent_options,
                                                       MDL);