tor

commit 7c70f713c31c0989a0008c7d0d92a1f12d498e32
parent 731a50c8c490c0d838605c2664ced3f68bb3d488
Author: Mike Perry <mikeperry-git@torproject.org>
Date:   Thu, 23 Mar 2023 20:52:13 +0000

Avoid closing dirty circs with active half-edges

In https://gitlab.torproject.org/tpo/core/tor/-/issues/40623, we changed the
DESTROY propogation to ensure memory was freed quickly at relays. This was a
good move, but it exacerbates the condition where a stream is closed on a
circuit, and then it is immediately closed because it is dirty. This creates a
race between the DESTROY and the last data sent on the stream. This race is
visible in shadow, and does happen.

This could be backported. A better solution to these kinds of problems is to
create an ENDED cell, and not close any circuits until the ENDED comes back.
But this will also require thinking, since this ENDED cell can also get lost,
so some kind of timeout may be needed either way. The ENDED cell could just
allow us to have much longer timeouts for this case.

Diffstat:
M
src/core/or/circuituse.c
 | 
1
+
M
src/core/or/connection_edge.c
 | 
19
+++++++++++++++++++
M
src/core/or/connection_edge.h
 | 
1
+

3 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c
@@ -1458,6 +1458,7 @@ circuit_expire_old_circuits_clientside(void)
     if (circ->timestamp_dirty &&
         circ->timestamp_dirty + get_options()->MaxCircuitDirtiness <
           now.tv_sec &&
+        !connection_half_edges_waiting(TO_ORIGIN_CIRCUIT(circ)) &&
         !TO_ORIGIN_CIRCUIT(circ)->p_streams /* nothing attached */ ) {
       log_debug(LD_CIRC, "Closing n_circ_id %u (dirty %ld sec ago, "
                 "purpose %d)",
diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c
@@ -675,6 +675,25 @@ connection_half_edge_add(const edge_connection_t *conn,
   smartlist_insert(circ->half_streams, insert_at, half_conn);
 }
 
+/**
+ * Return true if the circuit has any half-closed connections
+ * that are still within the end_ack_expected_usec timestamp
+ * from now.
+ */
+bool
+connection_half_edges_waiting(const origin_circuit_t *circ)
+{
+  if (!circ->half_streams)
+    return false;
+
+  SMARTLIST_FOREACH_BEGIN(circ->half_streams, const half_edge_t *, half_conn) {
+    if (half_conn->end_ack_expected_usec > monotime_absolute_usec())
+      return true;
+  } SMARTLIST_FOREACH_END(half_conn);
+
+  return false;
+}
+
 /** Release space held by <b>he</b> */
 void
 half_edge_free_(half_edge_t *he)
diff --git a/src/core/or/connection_edge.h b/src/core/or/connection_edge.h
@@ -218,6 +218,7 @@ int connection_half_edge_is_valid_end(smartlist_t *half_conns,
                                       streamid_t stream_id);
 int connection_half_edge_is_valid_resolved(smartlist_t *half_conns,
                                            streamid_t stream_id);
+bool connection_half_edges_waiting(const origin_circuit_t *circ);
 
 size_t half_streams_get_total_allocation(void);
 struct half_edge_t;