Merge branch 'bug40834' into 'main'

Revert "Nullify on_circuit if last conflux leg"

See merge request tpo/core/tor!744

4 files changed, 13 insertions, 47 deletions

diff --git a/changes/bug40834 b/changes/bug40834
new file mode 100644
index 0000000000..5c37e0c6a4
--- /dev/null
+++ b/changes/bug40834
@@ -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.
diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c
index eb004b3626..ab464f3446 100644
--- a/src/core/or/conflux.c
+++ b/src/core/or/conflux.c
@@ -467,8 +467,12 @@ conflux_decide_circ_for_send(conflux_t *cfx,
    * so these commands arrive in-order. */
   if (!new_circ && relay_command != RELAY_COMMAND_DATA) {
     /* Curr leg should be set, because conflux_decide_next_circ() should
-     * have set it earlier. */
-    tor_assert(cfx->curr_leg);
+     * have set it earlier. No BUG() here because the only caller BUG()s. */
+    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;
   }
 
diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c
index 837c1da720..79fd6c1648 100644
--- a/src/core/or/conflux_pool.c
+++ b/src/core/or/conflux_pool.c
@@ -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. */
 static void
 linked_nullify_streams(circuit_t *circ)
@@ -1592,7 +1549,6 @@ linked_circuit_closed(circuit_t *circ)
   if (CONFLUX_NUM_LEGS(circ->conflux) == 0) {
     /* Last leg, remove conflux object from linked set. */
     linked_pool_del(circ->conflux->nonce, is_client);
-    linked_detach_circuit(circ);
   } else {
     /* If there are other circuits, update streams backpointers and
      * nullify the stream lists. We do not free those streams in circuit_free_.
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 3af9435a76..893daa09fd 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -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,
                                         relay_command);
     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;
     } else {
       /* Conflux circuits always send multiplexed relay commands to