Fix: "Bug: Duplicate call to circuit_mark_for_close()"

Closes issue #40951

diff --git a/changes/bug40951 b/changes/bug40951
new file mode 100644 (file)
index 0000000..8ef87f3
--- /dev/null
+++ b/changes/bug40951
@@ -0,0 +1,4 @@
+  o Minor bugfixes (circuit handling):
+    - Prevent circuit_mark_for_close() from
+      being called twice on the same circuit.
+      Fixes bug 40951; bugfix on 0.4.8.16-dev.
\ No newline at end of file

diff --git a/src/core/or/command.c b/src/core/or/command.c
index cad7a173b6d650bd1ffb176f766cefecc86ec8d0..6e85311c865e24b55974e91ed1a008d00a8b6a3d 100644 (file)
--- a/src/core/or/command.c
+++ b/src/core/or/command.c
@@ -476,7 +476,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
 {
   const or_options_t *options = get_options();
   circuit_t *circ;
-  int reason, direction;
+  int direction;
   uint32_t orig_delivered_bw = 0;
   uint32_t orig_overhead_bw = 0;
 
@@ -566,7 +566,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
     }
   }
 
-  if ((reason = circuit_receive_relay_cell(cell, circ, direction)) < 0) {
+  if (circuit_receive_relay_cell(cell, circ, direction) < 0) {
     log_fn(LOG_DEBUG,LD_PROTOCOL,"circuit_receive_relay_cell "
            "(%s) failed. Closing.",
            direction==CELL_DIRECTION_OUT?"forward":"backward");
@@ -574,7 +574,6 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
     if (CIRCUIT_IS_ORIGIN(circ)) {
       control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ));
     }
-    circuit_mark_for_close(circ, -reason);
   }
 
   if (CIRCUIT_IS_ORIGIN(circ)) {

diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 005a597cf83fb307697950c045411a9c1c596ab0..a6321f180fe91c6e993c6059e0d2716a906c18c8 100644 (file)
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -227,7 +227,10 @@ circuit_update_channel_usage(circuit_t *circ, cell_t *cell)
  *  - If not recognized, then we need to relay it: append it to the appropriate
  *    cell_queue on <b>circ</b>.
  *
- * Return -<b>reason</b> on failure.
+ * If a reason exists to close <b>circ</b>, circuit_mark_for_close()
+ * is called in this function, so the caller doesn't have to do it.
+ *
+ * Return -<b>reason</b> on failure, else 0.
  */
 int
 circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
@@ -249,7 +252,8 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
       < 0) {
     log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
            "relay crypt failed. Dropping connection.");
-    return -END_CIRC_REASON_INTERNAL;
+    reason = -END_CIRC_REASON_INTERNAL;
+    goto error;
   }
 
   circuit_update_channel_usage(circ, cell);
@@ -280,10 +284,9 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
         log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
                "connection_edge_process_relay_cell (away from origin) "
                "failed.");
-        return reason;
+        goto error;
       }
-    }
-    if (cell_direction == CELL_DIRECTION_IN) {
+    } else if (cell_direction == CELL_DIRECTION_IN) {
       ++stats_n_relay_cells_delivered;
       log_debug(LD_OR,"Sending to origin.");
       reason = connection_edge_process_relay_cell(cell, circ, conn,
@@ -296,7 +299,7 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
           log_warn(LD_OR,
                    "connection_edge_process_relay_cell (at origin) failed.");
         }
-        return reason;
+        goto error;
       }
     }
     return 0;
@@ -319,7 +322,8 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
      * XXX: Shouldn't they always die? */
     if (circ->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) {
       TO_ORIGIN_CIRCUIT(circ)->path_state = PATH_STATE_USE_FAILED;
-      return -END_CIRC_REASON_TORPROTOCOL;
+      reason = -END_CIRC_REASON_TORPROTOCOL;
+      goto error;
     } else {
       return 0;
     }
@@ -339,15 +343,14 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
                                                CELL_DIRECTION_IN)) < 0) {
         log_warn(LD_REND, "Error relaying cell across rendezvous; closing "
                  "circuits");
-        /* XXXX Do this here, or just return -1? */
-        circuit_mark_for_close(circ, -reason);
-        return reason;
+        goto error;
       }
       return 0;
     }
     if (BUG(CIRCUIT_IS_ORIGIN(circ))) {
       /* Should be impossible at this point. */
-      return -END_CIRC_REASON_TORPROTOCOL;
+      reason = -END_CIRC_REASON_TORPROTOCOL;
+      goto error;
     }
     or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
     if (++or_circ->n_cells_discarded_at_end == 1) {
@@ -356,7 +359,8 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
              "Didn't recognize a cell, but circ stops here! Closing circuit. "
              "It was created %ld seconds ago.", (long)seconds_open);
     }
-    return -END_CIRC_REASON_TORPROTOCOL;
+    reason = -END_CIRC_REASON_TORPROTOCOL;
+    goto error;
   }
 
   log_debug(LD_OR,"Passing on unrecognized cell.");
@@ -366,9 +370,14 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
                                   * the cells. */
 
   if (append_cell_to_circuit_queue(circ, chan, cell, cell_direction, 0) < 0) {
-    return -END_CIRC_REASON_RESOURCELIMIT;
+    reason = -END_CIRC_REASON_RESOURCELIMIT;
+    goto error;
   }
   return 0;
+
+error:
+  circuit_mark_for_close(circ, -reason);
+  return reason;
 }
 
 /** Package a relay cell from an edge: