RELAY_COMMAND_XOFF in response to DNSPort resolve request = crash
Client-side resolve requests that originated via the DNSPort don't have conn->read_event set (you can recognize them because conn->is_dns_request is set instead). We have a special-case in connection_stop_reading() to avoid calling event_del() on the read_event when it's NULL, but we botched it so we don't actually avoid calling it. See #16248 for history, and in particular see how we put in the correct patch from cypherpunks in commit e79da6264: ``` + /* if dummy conn then no socket and no event, nothing to do here */ + if (conn->type == CONN_TYPE_AP && TO_EDGE_CONN(conn)->is_dns_request) { + tor_assert(!conn->read_event); + return; + } ``` but then in the next commit 91d7cf50 we refactored it to no longer return in the "is_dns_request 1, read_event NULL" case. Oops. (The attempted but failed bugfix went into 0.2.8.2-alpha, and the original and continuing bug was from 0.2.0.1-alpha when we added the DirPort.) But it gets worse! In commit 0422eb26 (Tor 0.4.7.2-alpha) we added the RELAY_COMMAND_XOFF cell, which lets the exit relay tell the client to call connection_stop_reading() on its streams. Specifically, handle_relay_msg() in src/core/or/relay.c calls circuit_process_stream_xoff() in src/core/or/congestion_control_flow.c which runs ``` log_info(LD_EDGE, "Got XOFF!"); connection_stop_reading(TO_CONN(conn)); conn->xoff_received = true; ``` Before 0.4.7.2-alpha the bug would only happen under high load and with poor luck, but after 0.4.7.2-alpha there is a way for the exit relay to trigger it on the client. (It has to correctly guess which streams to send them to, but getting a RESOLVE rather than a BEGIN seems like a good start.) Bug does not impact default Tors, and also doesn't impact Tor Browser. But the Tails and Whonix people will sure want to know.
issue