Merge branch 'dgoulet_ticket23709_033_01_squashed'

1 files changed, 24 insertions, 105 deletions

diff --git a/src/or/relay.c b/src/or/relay.c
index bbaf4ee785..0ad064417d 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -2746,7 +2746,13 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
       /* this code is duplicated from some of the logic below. Ugly! XXXX */
       tor_assert(destroy_queue->n > 0);
       cell = cell_queue_pop(destroy_queue);
-      channel_write_packed_cell(chan, cell);
+      /* Send the DESTROY cell. It is very unlikely that this fails but just
+       * in case, get rid of the channel. */
+      if (channel_write_packed_cell(chan, cell) < 0) {
+        /* The cell has been freed. */
+        channel_mark_for_close(chan);
+        continue;
+      }
       /* Update the cmux destroy counter */
       circuitmux_notify_xmit_destroy(cmux);
       cell = NULL;
@@ -2823,8 +2829,13 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
                                 DIRREQ_TUNNELED,
                                 DIRREQ_CIRC_QUEUE_FLUSHED);
 
-    /* Now send the cell */
-    channel_write_packed_cell(chan, cell);
+    /* Now send the cell. It is very unlikely that this fails but just in
+     * case, get rid of the channel. */
+    if (channel_write_packed_cell(chan, cell) < 0) {
+      /* The cell has been freed at this point. */
+      channel_mark_for_close(chan);
+      continue;
+    }
     cell = NULL;
 
     /*
@@ -2859,22 +2870,13 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
   return n_flushed;
 }
 
-#if 0
-/** Indicate the current preferred cap for middle circuits; zero disables
- * the cap.  Right now it's just a constant, ORCIRC_MAX_MIDDLE_CELLS, but
- * the logic in append_cell_to_circuit_queue() is written to be correct
- * if we want to base it on a consensus param or something that might change
- * in the future.
- */
-static int
-get_max_middle_cells(void)
-{
-  return ORCIRC_MAX_MIDDLE_CELLS;
-}
-#endif /* 0 */
-
 /** Add <b>cell</b> to the queue of <b>circ</b> writing to <b>chan</b>
- * transmitting in <b>direction</b>. */
+ * transmitting in <b>direction</b>.
+ *
+ * The given <b>cell</b> is copied over the circuit queue so the caller must
+ * cleanup the memory.
+ *
+ * This function is part of the fast path. */
 void
 append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
                              cell_t *cell, cell_direction_t direction,
@@ -2883,10 +2885,6 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
   or_circuit_t *orcirc = NULL;
   cell_queue_t *queue;
   int streams_blocked;
-#if 0
-  uint32_t tgt_max_middle_cells, p_len, n_len, tmp, hard_max_middle_cells;
-#endif
-
   int exitward;
   if (circ->marked_for_close)
     return;
@@ -2901,93 +2899,14 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
     streams_blocked = circ->streams_blocked_on_p_chan;
   }
 
-  /*
-   * Disabling this for now because of a possible guard discovery attack
-   */
-#if 0
-  /* Are we a middle circuit about to exceed ORCIRC_MAX_MIDDLE_CELLS? */
-  if ((circ->n_chan != NULL) && CIRCUIT_IS_ORCIRC(circ)) {
-    orcirc = TO_OR_CIRCUIT(circ);
-    if (orcirc->p_chan) {
-      /* We are a middle circuit if we have both n_chan and p_chan */
-      /* We'll need to know the current preferred maximum */
-      tgt_max_middle_cells = get_max_middle_cells();
-      if (tgt_max_middle_cells > 0) {
-        /* Do we need to initialize middle_max_cells? */
-        if (orcirc->max_middle_cells == 0) {
-          orcirc->max_middle_cells = tgt_max_middle_cells;
-        } else {
-          if (tgt_max_middle_cells > orcirc->max_middle_cells) {
-            /* If we want to increase the cap, we can do so right away */
-            orcirc->max_middle_cells = tgt_max_middle_cells;
-          } else if (tgt_max_middle_cells < orcirc->max_middle_cells) {
-            /*
-             * If we're shrinking the cap, we can't shrink past either queue;
-             * compare tgt_max_middle_cells rather than tgt_max_middle_cells *
-             * ORCIRC_MAX_MIDDLE_KILL_THRESH so the queues don't shrink enough
-             * to generate spurious warnings, either.
-             */
-            n_len = circ->n_chan_cells.n;
-            p_len = orcirc->p_chan_cells.n;
-            tmp = tgt_max_middle_cells;
-            if (tmp < n_len) tmp = n_len;
-            if (tmp < p_len) tmp = p_len;
-            orcirc->max_middle_cells = tmp;
-          }
-          /* else no change */
-        }
-      } else {
-        /* tgt_max_middle_cells == 0 indicates we should disable the cap */
-        orcirc->max_middle_cells = 0;
-      }
-
-      /* Now we know orcirc->max_middle_cells is set correctly */
-      if (orcirc->max_middle_cells > 0) {
-        hard_max_middle_cells =
-          (uint32_t)(((double)orcirc->max_middle_cells) *
-                     ORCIRC_MAX_MIDDLE_KILL_THRESH);
-
-        if ((unsigned)queue->n + 1 >= hard_max_middle_cells) {
-          /* Queueing this cell would put queue over the kill theshold */
-          log_warn(LD_CIRC,
-                   "Got a cell exceeding the hard cap of %u in the "
-                   "%s direction on middle circ ID %u on chan ID "
-                   U64_FORMAT "; killing the circuit.",
-                   hard_max_middle_cells,
-                   (direction == CELL_DIRECTION_OUT) ? "n" : "p",
-                   (direction == CELL_DIRECTION_OUT) ?
-                     circ->n_circ_id : orcirc->p_circ_id,
-                   U64_PRINTF_ARG(
-                     (direction == CELL_DIRECTION_OUT) ?
-                        circ->n_chan->global_identifier :
-                        orcirc->p_chan->global_identifier));
-          circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT);
-          return;
-        } else if ((unsigned)queue->n + 1 == orcirc->max_middle_cells) {
-          /* Only use ==, not >= for this test so we don't spam the log */
-          log_warn(LD_CIRC,
-                   "While trying to queue a cell, reached the soft cap of %u "
-                   "in the %s direction on middle circ ID %u "
-                   "on chan ID " U64_FORMAT ".",
-                   orcirc->max_middle_cells,
-                   (direction == CELL_DIRECTION_OUT) ? "n" : "p",
-                   (direction == CELL_DIRECTION_OUT) ?
-                     circ->n_circ_id : orcirc->p_circ_id,
-                   U64_PRINTF_ARG(
-                     (direction == CELL_DIRECTION_OUT) ?
-                        circ->n_chan->global_identifier :
-                        orcirc->p_chan->global_identifier));
-        }
-      }
-    }
-  }
-#endif /* 0 */
-
+  /* Very important that we copy to the circuit queue because all calls to
+   * this function use the stack for the cell memory. */
   cell_queue_append_packed_copy(circ, queue, exitward, cell,
                                 chan->wide_circ_ids, 1);
 
+  /* Check and run the OOM if needed. */
   if (PREDICT_UNLIKELY(cell_queues_check_size())) {
-    /* We ran the OOM handler */
+    /* We ran the OOM handler which might have closed this circuit. */
     if (circ->marked_for_close)
       return;
   }