From a382337be629ff74e621d2e24c8048e9e8d31d26 Mon Sep 17 00:00:00 2001
From: David Goulet <dgoulet@torproject.org>
Date: Wed, 11 Oct 2023 10:51:16 -0400
Subject: [PATCH] conflux: Flag set as in full teardown in the free path

We suspect a shutdown race of some sort for which the full teardown is not
noticed during the close but should be during the free.

For that, we flag the conflux set as in full teardown (if so) in the free path
in case the close path didn't caught it.

Fixes #40870

Signed-off-by: David Goulet <dgoulet@torproject.org>
---
 changes/ticket40870          |  4 ++++
 src/app/main/shutdown.c      |  1 +
 src/core/or/conflux_pool.c   | 43 +++++++++++++++++++++++++++++++++---
 src/core/or/conflux_pool.h   |  2 ++
 src/test/test_conflux_pool.c |  4 ++++
 5 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 changes/ticket40870

diff --git a/changes/ticket40870 b/changes/ticket40870
new file mode 100644
index 0000000000..c33c83e1a6
--- /dev/null
+++ b/changes/ticket40870
@@ -0,0 +1,4 @@
+  o Minor bugfixes (conflux, client):
+    - Avoid a non fatal assert caused by data coming in on a conflux set that is
+      being freed during shutdown. Fixes bug 40870; bugfix on 0.4.8.1-alpha.
+
diff --git a/src/app/main/shutdown.c b/src/app/main/shutdown.c
index b3f1c6d058..7f0d112c90 100644
--- a/src/app/main/shutdown.c
+++ b/src/app/main/shutdown.c
@@ -120,6 +120,7 @@ tor_free_all(int postfork)
   dirserv_free_all();
   rep_hist_free_all();
   bwhist_free_all();
+  conflux_notify_shutdown();
   circuit_free_all();
   conflux_pool_free_all();
   circpad_machines_free();
diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c
index 4a7e941372..ec2938aefc 100644
--- a/src/core/or/conflux_pool.c
+++ b/src/core/or/conflux_pool.c
@@ -1607,7 +1607,22 @@ linked_circuit_free(circuit_t *circ, bool is_client)
 
   /* Circuit can be freed without being closed and so we try to delete this leg
    * so we can learn if this circuit is the last leg or not. */
-  cfx_del_leg(circ->conflux, circ);
+  if (cfx_del_leg(circ->conflux, circ)) {
+    /* Check for instances of bug #40870, which we suspect happen
+     * during exit. If any happen outside of exit, BUG and warn. */
+    if (!circ->conflux->in_full_teardown) {
+      /* We should bug and warn if we're not in a shutdown process; that
+       * means we got here somehow without a close. */
+      if (BUG(!shutting_down)) {
+        log_warn(LD_BUG,
+                 "Conflux circuit %p being freed without being marked for "
+                 "full teardown via close, with shutdown state %d. "
+                 "Please report this.", circ, shutting_down);
+        conflux_log_set(LOG_WARN, circ->conflux, is_client);
+      }
+      circ->conflux->in_full_teardown = true;
+    }
+  }
 
   if (CONFLUX_NUM_LEGS(circ->conflux) > 0) {
     /* The last leg will free the streams but until then, we nullify to avoid
@@ -2126,14 +2141,36 @@ conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client)
   }
 }
 
+/**
+ * Conflux needs a notification when tor_shutdown() begins, so that
+ * when circuits are freed, new legs are not launched.
+ *
+ * This needs a separate notification from conflux_pool_free_all(),
+ * because circuits must be freed before that function.
+ */
+void
+conflux_notify_shutdown(void)
+{
+  shutting_down = true;
+}
+
+#ifdef TOR_UNIT_TESTS
+/**
+ * For unit tests: Clear the shutting down state so we resume building legs.
+ */
+void
+conflux_clear_shutdown(void)
+{
+  shutting_down = false;
+}
+#endif
+
 /** Free and clean up the conflux pool subsystem. This is called by the subsys
  * manager AFTER all circuits have been freed which implies that all objects in
  * the pools aren't referenced anymore. */
 void
 conflux_pool_free_all(void)
 {
-  shutting_down = true;
-
   digest256map_free(client_linked_pool, free_conflux_void_);
   digest256map_free(server_linked_pool, free_conflux_void_);
   digest256map_free(client_unlinked_pool, free_unlinked_void_);
diff --git a/src/core/or/conflux_pool.h b/src/core/or/conflux_pool.h
index afa4d9d058..eba726b03a 100644
--- a/src/core/or/conflux_pool.h
+++ b/src/core/or/conflux_pool.h
@@ -12,6 +12,7 @@
 #include "core/or/or.h"
 
 void conflux_pool_init(void);
+void conflux_notify_shutdown(void);
 void conflux_pool_free_all(void);
 
 origin_circuit_t *conflux_get_circ_for_conn(const entry_connection_t *conn,
@@ -41,6 +42,7 @@ void conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client);
 
 #ifdef TOR_UNIT_TESTS
 bool launch_new_set(int num_legs);
+void conflux_clear_shutdown(void);
 digest256map_t *get_linked_pool(bool is_client);
 digest256map_t *get_unlinked_pool(bool is_client);
 extern uint8_t DEFAULT_CLIENT_UX;
diff --git a/src/test/test_conflux_pool.c b/src/test/test_conflux_pool.c
index fc30677377..05dc7b14ff 100644
--- a/src/test/test_conflux_pool.c
+++ b/src/test/test_conflux_pool.c
@@ -396,6 +396,7 @@ test_setup(void)
 static void
 test_clear_circs(void)
 {
+  conflux_notify_shutdown();
   SMARTLIST_FOREACH(circ_pairs, circ_pair_t *, circ_pair, {
     tor_free(circ_pair);
   });
@@ -430,6 +431,9 @@ test_clear_circs(void)
   tor_assert(smartlist_len(mock_cell_delivery) == 0);
 
   (void)free_fake_origin_circuit;
+
+  /* Clear shutdown flag so we can resume testing again. */
+  conflux_clear_shutdown();
 }
 
 static void
-- 
2.47.3