[master] Fixed server crash after billing class is deleted

    Merges in rt39978.

diff --git a/RELNOTES b/RELNOTES
index a9862fab26b099406fb04c93cdacae4cf52790c8..d5c4227d29d4860388b244b04d7c1d28f7c9ce82 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -131,6 +131,11 @@ by Eric Young (eay@cryptsoft.com).
   Thanks to Jiri Popelka at Red Hat for the patch.
   [ISC-Bugs #36978]
 
+- 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.1-ESV-R11rc2
 
 - None

diff --git a/client/dhclient.c b/client/dhclient.c
index 5e016efa65ae6b145149d6a55dfbde864eee7be2..c55e221d05f647270d788975d0eb16b11d57adc0 100644 (file)
--- a/client/dhclient.c
+++ b/client/dhclient.c
@@ -820,11 +820,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 1f0b67c832401fe59602fe4071ea20b503c77a32..1bad3c7a11775cd99f612e58714dce80f673fbd8 100644 (file)
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -2768,7 +2768,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 2c5e685d50e4702509a51d80ae7f1502a25afbae..971f3ab20be0c1ebe334d140614cbe814dcafc62 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
  *
@@ -230,7 +230,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)
 {
@@ -246,24 +246,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)
@@ -274,7 +303,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 04e4e5e1acab3b1ca2dfe2c053e9c1fcca317ed9..63ff6213c3b5c6792192e8bc0cbc1c6f95b21acc 100644 (file)
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -1919,8 +1919,9 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
                                if (packet -> classes [i] ==
                                    lease -> billing_class)
                                        break;
+
                        if (i == packet -> class_count)
-                               unbill_class (lease, lease -> billing_class);
+                               unbill_class (lease);
                }
 
                /* If we don't have an active billing, see if we need
@@ -1956,7 +1957,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);
                }
        }
 

diff --git a/server/mdb.c b/server/mdb.c
index 19a120515ce2fa8f716c2c04a691574e557acaa3..75d7f42b34ce37efe91e1e364493648b7d145153 100644 (file)
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -1124,8 +1124,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);
        }
@@ -1442,8 +1442,8 @@ void make_binding_state_transition (struct lease *lease)
                                                          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);
@@ -1508,8 +1508,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);