Merge branch 'bug40834' into 'main'

Revert "Nullify on_circuit if last conflux leg"

See merge request tpo/core/tor!744
This commit is contained in:
David Goulet 2023-08-15 15:29:46 +00:00
commit c9dceca281
4 changed files with 13 additions and 47 deletions

5
changes/bug40834 Normal file
View File

@ -0,0 +1,5 @@
o Major bugfixes (conflux):
- Fix a relay-side crash caused by side effects of the fix for bug
40827. Reverts part of that fix that caused the crash and adds
additional log messages to help find the root cause. Fixes bug
40834; bugfix on 0.4.8.3-rc.

View File

@ -467,8 +467,12 @@ conflux_decide_circ_for_send(conflux_t *cfx,
* so these commands arrive in-order. */ * so these commands arrive in-order. */
if (!new_circ && relay_command != RELAY_COMMAND_DATA) { if (!new_circ && relay_command != RELAY_COMMAND_DATA) {
/* Curr leg should be set, because conflux_decide_next_circ() should /* Curr leg should be set, because conflux_decide_next_circ() should
* have set it earlier. */ * have set it earlier. No BUG() here because the only caller BUG()s. */
tor_assert(cfx->curr_leg); if (!cfx->curr_leg) {
log_warn(LD_BUG, "No current leg for conflux with relay command %d",
relay_command);
return NULL;
}
return cfx->curr_leg->circ; return cfx->curr_leg->circ;
} }

View File

@ -1509,49 +1509,6 @@ linked_update_stream_backpointers(circuit_t *circ)
} }
} }
/** This is called when this circuit is the last leg.
*
* The "on_circuit" pointer is nullified here so it is not given back to the
* conflux subsytem between the circuit mark for close step and actually
* freeing the circuit which is when streams are destroyed.
*
* Reason is that when the connection edge inbuf is packaged in
* connection_edge_package_raw_inbuf(), the on_circuit pointer is used and
* then passed on to conflux to learn which leg to use. However, if the circuit
* was marked prior but not yet freed, there are no more legs remaining which
* leads to a linked circuit being used without legs. No bueno. */
static void
linked_detach_circuit(circuit_t *circ)
{
tor_assert(circ);
tor_assert_nonfatal(circ->conflux);
if (CIRCUIT_IS_ORIGIN(circ)) {
origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
tor_assert_nonfatal(circ->purpose == CIRCUIT_PURPOSE_CONFLUX_LINKED);
/* Iterate over stream list using next_stream pointer, until null */
for (edge_connection_t *stream = ocirc->p_streams; stream;
stream = stream->next_stream) {
/* Update the circuit pointer of each stream */
stream->on_circuit = NULL;
}
} else {
or_circuit_t *orcirc = TO_OR_CIRCUIT(circ);
/* Iterate over stream list using next_stream pointer, until null */
for (edge_connection_t *stream = orcirc->n_streams; stream;
stream = stream->next_stream) {
/* Update the circuit pointer of each stream */
stream->on_circuit = NULL;
}
/* Iterate over stream list using next_stream pointer, until null */
for (edge_connection_t *stream = orcirc->resolving_streams; stream;
stream = stream->next_stream) {
/* Update the circuit pointer of each stream */
stream->on_circuit = NULL;
}
}
}
/** Nullify all streams of the given circuit. */ /** Nullify all streams of the given circuit. */
static void static void
linked_nullify_streams(circuit_t *circ) linked_nullify_streams(circuit_t *circ)
@ -1592,7 +1549,6 @@ linked_circuit_closed(circuit_t *circ)
if (CONFLUX_NUM_LEGS(circ->conflux) == 0) { if (CONFLUX_NUM_LEGS(circ->conflux) == 0) {
/* Last leg, remove conflux object from linked set. */ /* Last leg, remove conflux object from linked set. */
linked_pool_del(circ->conflux->nonce, is_client); linked_pool_del(circ->conflux->nonce, is_client);
linked_detach_circuit(circ);
} else { } else {
/* If there are other circuits, update streams backpointers and /* If there are other circuits, update streams backpointers and
* nullify the stream lists. We do not free those streams in circuit_free_. * nullify the stream lists. We do not free those streams in circuit_free_.

View File

@ -639,7 +639,8 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *orig_circ,
circ = conflux_decide_circ_for_send(orig_circ->conflux, orig_circ, circ = conflux_decide_circ_for_send(orig_circ->conflux, orig_circ,
relay_command); relay_command);
if (BUG(!circ)) { if (BUG(!circ)) {
log_warn(LD_BUG, "No circuit to send on for conflux"); log_warn(LD_BUG, "No circuit to send for conflux for relay command %d, "
"called from %s:%d", relay_command, filename, lineno);
circ = orig_circ; circ = orig_circ;
} else { } else {
/* Conflux circuits always send multiplexed relay commands to /* Conflux circuits always send multiplexed relay commands to