isisd: fix circuit state machine

isisd has a so-called circuit state machine that takes care about
the interface state changes, such as initializing, down, up. When
an interface was brought down by a link failure, the interface
information was deleted and set to NULL. When the link was restored
later, the interface was looked up by the old pointer, but since it
was cleared, it was never found again, resulting in an interface
never entering the up state again.

Also, the program regularly crashed because of a deleted pointer in
the same context which was later accessed without any further
checking.

Signed-off-by: Fritz Reichmann <fritz@reichmann.nl>

diff --git a/isisd/isis_adjacency.c b/isisd/isis_adjacency.c
index aab8d1a3dcede78466332046489a1fc33908ca70..de34bea9d3f8b73943533eec10f5f14e86748080 100644 (file)
--- a/isisd/isis_adjacency.c
+++ b/isisd/isis_adjacency.c
@@ -172,7 +172,7 @@ isis_adj_state_change (struct isis_adjacency *adj, enum isis_adj_state state,
        circuit->upadjcount[level - 1]++;
       if (state == ISIS_ADJ_DOWN)
        {
-         isis_delete_adj (adj, adj->circuit->u.bc.adjdb[level - 1]);
+         listnode_delete (adj->circuit->u.bc.adjdb[level - 1], adj);
          circuit->upadjcount[level - 1]--;
        }
 

diff --git a/isisd/isis_circuit.h b/isisd/isis_circuit.h
index a7e719f6054998107c32bab93a7f068063889cd4..f32d1ddae709eb0bda05884d6bcf6e062181946a 100644 (file)
--- a/isisd/isis_circuit.h
+++ b/isisd/isis_circuit.h
@@ -65,6 +65,7 @@ struct isis_p2p_info
 struct isis_circuit
 {
   int state;
+  int connected;
   u_char circuit_id;           /* l1/l2 p2p/bcast CircuitID */
   struct isis_area *area;      /* back pointer to the area */
   struct interface *interface; /* interface info from z */

diff --git a/isisd/isis_csm.c b/isisd/isis_csm.c
index 80d0c9066340c33e1ff2dcfe9970111164e7c470..6cdde46a9875956c3e57a14c0a29c7d9b0451808 100644 (file)
--- a/isisd/isis_csm.c
+++ b/isisd/isis_csm.c
@@ -112,6 +112,7 @@ isis_csm_state_change (int event, struct isis_circuit *circuit, void *arg)
          isis_circuit_configure (circuit, (struct isis_area *) arg);
          isis_circuit_up (circuit);
          circuit->state = C_STATE_UP;
+         circuit->connected = 1;
          isis_event_circuit_state_change (circuit, 1);
          listnode_delete (isis->init_circ_list, circuit);
          break;
@@ -136,9 +137,12 @@ isis_csm_state_change (int event, struct isis_circuit *circuit, void *arg)
          zlog_warn ("circuit already enabled");
          break;
        case IF_UP_FROM_Z:
-         isis_circuit_if_add (circuit, (struct interface *) arg);
-         isis_circuit_up (circuit);
+         if (!circuit->connected) {
+           isis_circuit_if_add (circuit, (struct interface *) arg);
+           isis_circuit_up (circuit);
+         }
          circuit->state = C_STATE_UP;
+         circuit->connected = 1;
          isis_event_circuit_state_change (circuit, 1);
          break;
        case ISIS_DISABLE:
@@ -167,7 +171,6 @@ isis_csm_state_change (int event, struct isis_circuit *circuit, void *arg)
          isis_event_circuit_state_change (circuit, 0);
          break;
        case IF_DOWN_FROM_Z:
-         isis_circuit_if_del (circuit);
          circuit->state = C_STATE_CONF;
          isis_event_circuit_state_change (circuit, 0);
          break;

diff --git a/isisd/isis_lsp.c b/isisd/isis_lsp.c
index e12e4ca8ea42b1b4465e792ef512100202903426..9db0db9da38e2dba8c047b1ee9e94a1331f68376 100644 (file)
--- a/isisd/isis_lsp.c
+++ b/isisd/isis_lsp.c
@@ -2037,6 +2037,8 @@ lsp_tick (struct thread *thread)
            {
               for (ALL_LIST_ELEMENTS_RO (area->circuit_list, cnode, circuit))
                {
+                 if (circuit->state != C_STATE_UP)
+                   continue;
                   for (ALL_LIST_ELEMENTS_RO (lsp_list, lspnode, lsp))
                    {
                      if (ISIS_CHECK_FLAG (lsp->SRMflags, circuit))

diff --git a/isisd/isis_pdu.c b/isisd/isis_pdu.c
index 0896d540c3af20e134d46572d43f7078d3bd1841..4c602eea76fea963564c499494554d09f266f6d2 100644 (file)
--- a/isisd/isis_pdu.c
+++ b/isisd/isis_pdu.c
@@ -1916,6 +1916,9 @@ send_hello (struct isis_circuit *circuit, int level)
   unsigned long len_pointer, length;
   int retval;
 
+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
   if (circuit->interface->mtu == 0)
     {
       zlog_warn ("circuit has zero MTU");
@@ -2222,6 +2225,9 @@ send_csnp (struct isis_circuit *circuit, int level)
   struct listnode *node;
   struct isis_lsp *lsp;
 
+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
   memset (start, 0x00, ISIS_SYS_ID_LEN + 2);
   memset (stop, 0xff, ISIS_SYS_ID_LEN + 2);
 
@@ -2387,6 +2393,9 @@ send_psnp (int level, struct isis_circuit *circuit)
   struct list *list = NULL;
   struct listnode *node;
 
+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
   if ((circuit->circ_type == CIRCUIT_T_BROADCAST &&
        !circuit->u.bc.is_dr[level - 1]) ||
       circuit->circ_type != CIRCUIT_T_BROADCAST)
@@ -2493,85 +2502,85 @@ send_lsp (struct thread *thread)
   circuit = THREAD_ARG (thread);
   assert (circuit);
 
-  if (circuit->state == C_STATE_UP)
+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
+  lsp = listgetdata ((node = listhead (circuit->lsp_queue)));
+
+  /*
+   * Do not send if levels do not match
+   */
+  if (!(lsp->level & circuit->circuit_is_type))
+    goto dontsend;
+
+  /*
+   * Do not send if we do not have adjacencies in state up on the circuit
+   */
+  if (circuit->upadjcount[lsp->level - 1] == 0)
+    goto dontsend;
+  /* only send if it needs sending */
+  if ((time (NULL) - lsp->last_sent) >=
+      circuit->area->lsp_gen_interval[lsp->level - 1])
     {
-      lsp = listgetdata ((node = listhead (circuit->lsp_queue)));
 
-      /*
-       * Do not send if levels do not match
-       */
-      if (!(lsp->level & circuit->circuit_is_type))
-       goto dontsend;
+      if (isis->debugs & DEBUG_UPDATE_PACKETS)
+       {
+         zlog_debug
+           ("ISIS-Upd (%s): Sent L%d LSP %s, seq 0x%08x, cksum 0x%04x,"
+            " lifetime %us on %s", circuit->area->area_tag, lsp->level,
+            rawlspid_print (lsp->lsp_header->lsp_id),
+            ntohl (lsp->lsp_header->seq_num),
+            ntohs (lsp->lsp_header->checksum),
+            ntohs (lsp->lsp_header->rem_lifetime),
+            circuit->interface->name);
+       }
+       /* copy our lsp to the send buffer */
+       stream_copy (circuit->snd_stream, lsp->pdu);
+
+       retval = circuit->tx (circuit, lsp->level);
 
       /*
-       * Do not send if we do not have adjacencies in state up on the circuit
+       * If the sending succeeded, we can del the lsp from circuits
+       * lsp_queue
        */
-      if (circuit->upadjcount[lsp->level - 1] == 0)
-       goto dontsend;
-      /* only send if it needs sending */
-      if ((time (NULL) - lsp->last_sent) >=
-         circuit->area->lsp_gen_interval[lsp->level - 1])
+      if (retval == ISIS_OK)
        {
-
-         if (isis->debugs & DEBUG_UPDATE_PACKETS)
-           {
-             zlog_debug
-               ("ISIS-Upd (%s): Sent L%d LSP %s, seq 0x%08x, cksum 0x%04x,"
-                " lifetime %us on %s", circuit->area->area_tag, lsp->level,
-                rawlspid_print (lsp->lsp_header->lsp_id),
-                ntohl (lsp->lsp_header->seq_num),
-                ntohs (lsp->lsp_header->checksum),
-                ntohs (lsp->lsp_header->rem_lifetime),
-                circuit->interface->name);
-           }
-         /* copy our lsp to the send buffer */
-         stream_copy (circuit->snd_stream, lsp->pdu);
-
-         retval = circuit->tx (circuit, lsp->level);
+         list_delete_node (circuit->lsp_queue, node);
 
          /*
-          * If the sending succeeded, we can del the lsp from circuits
-          * lsp_queue
+          * On broadcast circuits also the SRMflag can be cleared
           */
-         if (retval == ISIS_OK)
-           {
-             list_delete_node (circuit->lsp_queue, node);
+         if (circuit->circ_type == CIRCUIT_T_BROADCAST)
+           ISIS_CLEAR_FLAG (lsp->SRMflags, circuit);
 
+         if (flags_any_set (lsp->SRMflags) == 0)
+           {
              /*
-              * On broadcast circuits also the SRMflag can be cleared
+              * need to remember when we were last sent
               */
-             if (circuit->circ_type == CIRCUIT_T_BROADCAST)
-               ISIS_CLEAR_FLAG (lsp->SRMflags, circuit);
-
-             if (flags_any_set (lsp->SRMflags) == 0)
-               {
-                 /*
-                  * need to remember when we were last sent
-                  */
-                 lsp->last_sent = time (NULL);
-               }
-           }
-         else
-           {
-             zlog_debug ("sending of level %d link state failed", lsp->level);
+             lsp->last_sent = time (NULL);
            }
        }
       else
        {
-         /* my belief is that if it wasn't his time, the lsp can be removed
-          * from the queue
-          */
-       dontsend:
-         list_delete_node (circuit->lsp_queue, node);
+         zlog_debug ("sending of level %d link state failed", lsp->level);
        }
-#if 0
-      /*
-       * If there are still LSPs send next one after lsp-interval (33 msecs)
+    }
+  else
+    {
+      /* my belief is that if it wasn't his time, the lsp can be removed
+       * from the queue
        */
-      if (listcount (circuit->lsp_queue) > 0)
-       thread_add_timer (master, send_lsp, circuit, 1);
-#endif
+    dontsend:
+      list_delete_node (circuit->lsp_queue, node);
     }
+#if 0
+  /*
+   * If there are still LSPs send next one after lsp-interval (33 msecs)
+   */
+  if (listcount (circuit->lsp_queue) > 0)
+    thread_add_timer (master, send_lsp, circuit, 1);
+#endif
 
   return retval;
 }