[tor-bugs] #25226 [Core Tor/Tor]: Circuit cell queue can fill up memory

[Tor Bug Tracker & Wiki]

#25226: Circuit cell queue can fill up memory
------------------------------------------+--------------------------------
 Reporter:  dgoulet                       |          Owner:  (none)
     Type:  defect                        |         Status:
                                          |  needs_information
 Priority:  Medium                        |      Milestone:  Tor:
                                          |  0.3.3.x-final
Component:  Core Tor/Tor                  |        Version:
 Severity:  Normal                        |     Resolution:
 Keywords:  tor-cell, tor-relay, tor-dos  |  Actual Points:
Parent ID:                                |         Points:
 Reviewer:                                |        Sponsor:
------------------------------------------+--------------------------------
Changes (by dgoulet):

 * status:  new => needs_information


Comment:

 This is *not* trivial so I'll try to explain what I can see:

 In `append_cell_to_circuit_queue()`, there is a check on the cell queue
 for a maximum size which then makes the circuit to stop reading on the
 connection if reached:

 {{{
   /* If we have too many cells on the circuit, we should stop reading from
    * the edge streams for a while. */
   if (!streams_blocked && queue->n >= CELL_QUEUE_HIGHWATER_SIZE)
     set_streams_blocked_on_circ(circ, chan, 1, 0); /* block streams */
 }}}

 In `set_streams_blocked_on_circ()` there is this non documented non
 trivial if as the very first thing it does:

 {{{
   if (circ->n_chan == chan) {
     circ->streams_blocked_on_n_chan = block;
     if (CIRCUIT_IS_ORIGIN(circ))
       edge = TO_ORIGIN_CIRCUIT(circ)->p_streams;
   } else {
     circ->streams_blocked_on_p_chan = block;
     tor_assert(!CIRCUIT_IS_ORIGIN(circ));
     edge = TO_OR_CIRCUIT(circ)->n_streams;
   }

   [stop reading on all the "edge" streams]
 }}}

 Lets use the example where we are a Guard and we have an `or_circuit_t`
 with the "p_chan" being the client connection and the "n_chan" being the
 connection to the middle node.

 If we set the block on `n_chan`, we would only stop the read() if the
 circuit is origin because `p_streams` is only set on a tor client.

 Does this means that if we try to deliver a cell forward (n_chan), even
 though we've reached the high limit, we'll still queue it because there is
 never a time where we check if we are blocked on "n_chan" at the relay
 level?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25226#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online