- Fixed a bug that leaks host record references onto lease structures,

  causing the server to apply configuration intended for one host to any
  other innocent clients that come along later.  [ISC-Bugs #22018]

diff --git a/RELNOTES b/RELNOTES
index df1857c6877d2e598741aec09a510245a9ecdbfd..62b296d0f461c223654d84633d24082c4dde3edc 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -67,6 +67,10 @@ work on other platforms. Please report any problems and suggested fixes to
 - Add code to clear the pointer to an object in an OMAPI handle when the
   object is freed due to a dereference.  [ISC-Bugs #21306]
 
+- Fixed a bug that leaks host record references onto lease structures,
+  causing the server to apply configuration intended for one host to any
+  other innocent clients that come along later.  [ISC-Bugs #22018]
+
                        Changes since 4.2.0b2
 
 - Add declaration for variable in debug code in alloc.c.  [ISC-Bugs #21472]

diff --git a/server/dhcp.c b/server/dhcp.c
index 3dd95ae36327cf67bebf8544945d8dfa15e7cea9..8fbd7337f5cbdaacbe60579c3b77d77206c7dbc6 100644 (file)
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -3848,26 +3848,46 @@ int find_lease (struct lease **lp,
                lease_dereference (&hw_lease, MDL);
        }
 
-       /* If we found a host_decl but no matching address, try to
-          find a host_decl that has no address, and if there is one,
-          hang it off the lease so that we can use the supplied
-          options. */
-       if (lease && host && !lease -> host) {
-               struct host_decl *p = (struct host_decl *)0;
-               struct host_decl *n = (struct host_decl *)0;
-               host_reference (&p, host, MDL);
-               while (p) {
-                       if (!p -> fixed_addr) {
-                               host_reference (&lease -> host, p, MDL);
-                               host_dereference (&p, MDL);
+       /*
+        * If we found a host_decl but no matching address, try to
+        * find a host_decl that has no address, and if there is one,
+        * hang it off the lease so that we can use the supplied
+        * options.
+        */
+       if (lease && host && !lease->host) {
+               struct host_decl *p = NULL;
+               struct host_decl *n = NULL;
+
+               host_reference(&p, host, MDL);
+               while (p != NULL) {
+                       if (!p->fixed_addr) {
+                               /*
+                                * If the lease is currently active, then it
+                                * must be allocated to the present client.
+                                * We store a reference to the host record on
+                                * the lease to save a lookup later (in
+                                * ack_lease()).  We mustn't refer to the host
+                                * record on non-active leases because the
+                                * client may be denied later.
+                                *
+                                * XXX: Not having this reference (such as in
+                                * DHCPDISCOVER/INIT) means ack_lease will have
+                                * to perform this lookup a second time.  This
+                                * hopefully isn't a problem as DHCPREQUEST is
+                                * more common than DHCPDISCOVER.
+                                */
+                               if (lease->binding_state == FTS_ACTIVE)
+                                       host_reference(&lease->host, p, MDL);
+
+                               host_dereference(&p, MDL);
                                break;
                        }
-                       if (p -> n_ipaddr)
-                               host_reference (&n, p -> n_ipaddr, MDL);
-                       host_dereference (&p, MDL);
-                       if (n) {
-                               host_reference (&p, n, MDL);
-                               host_dereference (&n, MDL);
+                       if (p->n_ipaddr != NULL)
+                               host_reference(&n, p->n_ipaddr, MDL);
+                       host_dereference(&p, MDL);
+                       if (n != NULL) {
+                               host_reference(&p, n, MDL);
+                               host_dereference(&n, MDL);
                        }
                }
        }
@@ -4104,10 +4124,23 @@ int allocate_lease (struct lease **lp, struct packet *packet,
                        lease = candl;
        }
 
-       if (lease) {
-               if (lease -> binding_state == FTS_ABANDONED)
-                       log_error ("Reclaiming abandoned lease %s.",
-                                  piaddr (lease -> ip_addr));
+       if (lease != NULL) {
+               if (lease->binding_state == FTS_ABANDONED)
+                       log_error("Reclaiming abandoned lease %s.",
+                                 piaddr(lease->ip_addr));
+
+               /*
+                * XXX: For reliability, we go ahead and remove the host
+                * record and try to move on.  For correctness, if there
+                * are any other stale host vectors, we want to find them.
+                */
+               if (lease->host != NULL) {
+                       log_debug("soft impossible condition (%s:%d): stale "
+                                 "host \"%s\" found on lease %s", MDL,
+                                 lease->host->name,
+                                 piaddr(lease->ip_addr));
+                       host_dereference(&lease->host, MDL);
+               }
 
                lease_reference (lp, lease, MDL);
                return 1;