aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/ticket409213
-rw-r--r--src/core/or/circuitbuild.c6
-rw-r--r--src/core/or/conflux.c5
-rw-r--r--src/core/or/relay.c51
-rw-r--r--src/core/or/relay.h6
-rw-r--r--src/feature/relay/circuitbuild_relay.c6
6 files changed, 55 insertions, 22 deletions
diff --git a/changes/ticket40921 b/changes/ticket40921
new file mode 100644
index 0000000000..5818b91864
--- /dev/null
+++ b/changes/ticket40921
@@ -0,0 +1,3 @@
+ o Minor bugfixes (conflux):
+ - Avoid a potential hard assert (crash) when sending a cell on a Conflux
+ set. Fixes bug 40921; bugfix on 0.4.8.1-alpha.
diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c
index d6e022e7fa..dc1912294b 100644
--- a/src/core/or/circuitbuild.c
+++ b/src/core/or/circuitbuild.c
@@ -816,8 +816,10 @@ circuit_deliver_create_cell,(circuit_t *circ,
circuit_set_n_circid_chan(circ, id, circ->n_chan);
cell.circ_id = circ->n_circ_id;
- append_cell_to_circuit_queue(circ, circ->n_chan, &cell,
- CELL_DIRECTION_OUT, 0);
+ if (append_cell_to_circuit_queue(circ, circ->n_chan, &cell,
+ CELL_DIRECTION_OUT, 0) < 0) {
+ return -1;
+ }
if (CIRCUIT_IS_ORIGIN(circ)) {
/* Update began timestamp for circuits starting their first hop */
diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c
index 677df95067..9025dcffaf 100644
--- a/src/core/or/conflux.c
+++ b/src/core/or/conflux.c
@@ -531,7 +531,10 @@ conflux_note_cell_sent(conflux_t *cfx, circuit_t *circ, uint8_t relay_command)
}
leg = conflux_get_leg(cfx, circ);
- tor_assert(leg);
+ if (leg == NULL) {
+ log_fn(LOG_PROTOCOL_WARN, LD_BUG, "No Conflux leg after sending a cell");
+ return;
+ }
leg->last_seq_sent++;
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 6abe802355..8e6fddf18b 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -365,14 +365,19 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
* we might kill the circ before we relay
* the cells. */
- append_cell_to_circuit_queue(circ, chan, cell, cell_direction, 0);
+ if (append_cell_to_circuit_queue(circ, chan, cell, cell_direction, 0) < 0) {
+ return -END_CIRC_REASON_RESOURCELIMIT;
+ }
return 0;
}
/** Package a relay cell from an edge:
* - Encrypt it to the right layer
* - Append it to the appropriate cell_queue on <b>circ</b>.
- */
+ *
+ * Return 1 if the cell was successfully sent as in queued on the circuit.
+ * Return 0 if the cell needs to be dropped as in ignored.
+ * Return -1 on error for which the circuit should be marked for close. */
MOCK_IMPL(int,
circuit_package_relay_cell, (cell_t *cell, circuit_t *circ,
cell_direction_t cell_direction,
@@ -430,8 +435,8 @@ circuit_package_relay_cell, (cell_t *cell, circuit_t *circ,
}
++stats_n_relay_cells_relayed;
- append_cell_to_circuit_queue(circ, chan, cell, cell_direction, on_stream);
- return 0;
+ return append_cell_to_circuit_queue(circ, chan, cell,
+ cell_direction, on_stream);
}
/** If cell's stream_id matches the stream_id of any conn that's
@@ -742,13 +747,24 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *orig_circ,
circuit_sent_valid_data(origin_circ, rh.length);
}
- if (circuit_package_relay_cell(&cell, circ, cell_direction, cpath_layer,
- stream_id, filename, lineno) < 0) {
+ int ret = circuit_package_relay_cell(&cell, circ, cell_direction,
+ cpath_layer, stream_id, filename,
+ lineno);
+ if (ret < 0) {
log_warn(LD_BUG,"circuit_package_relay_cell failed. Closing.");
circuit_mark_for_close(circ, END_CIRC_REASON_INTERNAL);
return -1;
+ } else if (ret == 0) {
+ /* This means we should drop the cell or that the circuit was already
+ * marked for close. At this point in time, we do NOT close the circuit if
+ * the cell is dropped. It is not the case with arti where each circuit
+ * protocol violation will lead to closing the circuit. */
+ return 0;
}
+ /* At this point, we are certain that the cell was queued on the circuit and
+ * thus will be sent on the wire. */
+
if (circ->conflux) {
conflux_note_cell_sent(circ->conflux, circ, relay_command);
}
@@ -3381,8 +3397,13 @@ relay_consensus_has_changed(const networkstatus_t *ns)
* The given <b>cell</b> is copied onto the circuit queue so the caller must
* cleanup the memory.
*
- * This function is part of the fast path. */
-void
+ * This function is part of the fast path.
+ *
+ * Return 1 if the cell was successfully sent.
+ * Return 0 if the cell can not be sent. The caller MUST NOT close the circuit.
+ * Return -1 indicating an error and that the caller should mark the circuit
+ * for close. */
+int
append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
cell_t *cell, cell_direction_t direction,
streamid_t fromstream)
@@ -3393,8 +3414,9 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
int32_t max_queue_size;
int circ_blocked;
int exitward;
- if (circ->marked_for_close)
- return;
+ if (circ->marked_for_close) {
+ return 0;
+ }
exitward = (direction == CELL_DIRECTION_OUT);
if (exitward) {
@@ -3424,9 +3446,8 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
"Closing circuit for safety reasons.",
(exitward) ? "Outbound" : "Inbound", queue->n,
max_queue_size);
- circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT);
stats_n_circ_max_cell_reached++;
- return;
+ return -1;
}
/* Very important that we copy to the circuit queue because all calls to
@@ -3437,8 +3458,9 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
/* Check and run the OOM if needed. */
if (PREDICT_UNLIKELY(cell_queues_check_size())) {
/* We ran the OOM handler which might have closed this circuit. */
- if (circ->marked_for_close)
- return;
+ if (circ->marked_for_close) {
+ return 0;
+ }
}
/* If we have too many cells on the circuit, note that it should
@@ -3462,6 +3484,7 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
/* New way: mark this as having waiting cells for the scheduler */
scheduler_channel_has_waiting_cells(chan);
+ return 1;
}
/** Append an encoded value of <b>addr</b> to <b>payload_out</b>, which must
diff --git a/src/core/or/relay.h b/src/core/or/relay.h
index 3a1f3b0ae5..12198ae982 100644
--- a/src/core/or/relay.h
+++ b/src/core/or/relay.h
@@ -76,9 +76,9 @@ void cell_queue_append_packed_copy(circuit_t *circ, cell_queue_t *queue,
int exitward, const cell_t *cell,
int wide_circ_ids, int use_stats);
-void append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
- cell_t *cell, cell_direction_t direction,
- streamid_t fromstream);
+int append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
+ cell_t *cell, cell_direction_t direction,
+ streamid_t fromstream);
void destroy_cell_queue_init(destroy_cell_queue_t *queue);
void destroy_cell_queue_clear(destroy_cell_queue_t *queue);
diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c
index 5b1609a1af..ce6cbe6df4 100644
--- a/src/feature/relay/circuitbuild_relay.c
+++ b/src/feature/relay/circuitbuild_relay.c
@@ -579,8 +579,10 @@ onionskin_answer(struct or_circuit_t *circ,
int used_create_fast = (created_cell->cell_type == CELL_CREATED_FAST);
- append_cell_to_circuit_queue(TO_CIRCUIT(circ),
- circ->p_chan, &cell, CELL_DIRECTION_IN, 0);
+ if (append_cell_to_circuit_queue(TO_CIRCUIT(circ), circ->p_chan,
+ &cell, CELL_DIRECTION_IN, 0) < 0) {
+ return -1;
+ }
log_debug(LD_CIRC,"Finished sending '%s' cell.",
used_create_fast ? "created_fast" : "created");
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion