diff options
author | Thomas Markwalder <tmark@isc.org> | 2015-07-29 08:32:50 -0400 |
---|---|---|
committer | Thomas Markwalder <tmark@isc.org> | 2015-07-29 08:32:50 -0400 |
commit | 6a39bcf0be695fa2e0b62312ea8bdc830a08f7bc (patch) | |
tree | fd823fce88d2d70954c8c0a7be9f93d7b6736144 | |
parent | 673137b13addbcb5b8f672f6d905e979a02fa69e (diff) | |
download | isc-dhcp-6a39bcf0be695fa2e0b62312ea8bdc830a08f7bc.tar.gz |
[master] Fixed server crash after billing class is deleted
Merges in rt39978.
-rw-r--r-- | RELNOTES | 5 | ||||
-rw-r--r-- | client/dhclient.c | 4 | ||||
-rw-r--r-- | includes/dhcpd.h | 2 | ||||
-rw-r--r-- | server/class.c | 57 | ||||
-rw-r--r-- | server/dhcp.c | 4 | ||||
-rw-r--r-- | server/mdb.c | 12 |
6 files changed, 58 insertions, 26 deletions
@@ -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 d14a9cf7..be2af6ad 100644 --- 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 4e75421e..20bf30e9 100644 --- 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 48d4a6cb..5c1d8e73 100644 --- 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 5db75f75..1f007e29 100644 --- 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 6a637b5e..219db506 100644 --- 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); |