[tor-bugs] #10169 [Tor]: Extend OOM handler to cover channels/connection buffers

[Tor Bug Tracker & Wiki]

#10169: Extend OOM handler to cover channels/connection buffers
------------------------+----------------------------------------
     Reporter:  nickm   |      Owner:
         Type:  defect  |     Status:  needs_review
     Priority:  major   |  Milestone:  Tor: 0.2.5.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-relay oom 024-backport
Actual Points:          |  Parent ID:
       Points:          |
------------------------+----------------------------------------

Comment (by nickm):

 Replying to [comment:10 sysrqb]:
 > Reviewed bug10169_024.
 >
 > total_bytes_allocated_in_chunks still has a DOCDOC in buffers.c

 Will fix later in 0.2.5.

 > I wonder if END_CIRC_REASON_RESOURCELIMIT can be used to perform a
 modified sniper-based oracle attack if there's another way to fill the
 remaining 10% when the 90% memory usage is reached.

 The idea being, cause a node to go nearly OOM, and then see which streams
 (as a client!) got END_STREAM_REASON_RESOURCELIMIT, so you know that
 you're nearly at the OOM point, and then somehow make the node consume
 another .1 * MaxMem ?

 If that's what you meant, it would work, but I think it only means that
 MaxMem needs to be set conservatively, and we need to be on the lookout
 for other ways to pump up a node's memory consumption.  Even if we didn't
 send END_STREAM_REASON_RESOURCELIMIT, an attacker could still snipe a node
 if they know a way to make it run out of memory without its buffers and
 cell queues exceeding .9*MaxMem.

 > It might be useful to the relay operator if Tor says how many circuits
 remained alive after circuits_handle_oom runs, e.g. modify the log notice
 at the end of that function.
 >
 > {{{
 >   int n_circuits_alive = smartlist_len(circlist) - n_circuits_killed;
 >   log_notice(LD_GENERAL, "Removed "U64_FORMAT" bytes by killing %d "
 >                          "circuits, %d circuits remain alive.",
 >              U64_PRINTF_ARG(mem_recovered), n_circuits_killed,
 >              n_circuits_alive);
 > }}}

 Done in 79c234e0e3fa22d76029bd3b5e2c52072709cff3

 > Maybe the comments for circuit_get_streams_max_data_age and
 marked_circuit_streams_free_bytes should notate that they are helpers for
 circuit_max_queued_data_age and marked_circuit_free_stream_bytes,
 respectively?
 >
 > Is it possible that when the stream buffers are "aggressively freed"
 using chunk_free_unchecked() they may not actually be freed, but prepended
 to a freelist and, as a result, less memory is actually freed in
 circuits_handle_oom() than is expected?

 I think I fixed that in fd28754dd3dce0e00304825d531348414c0a354b

 The branches to review are still bug10169_023 (for the main patch),
 bug10169_024 (for the merge), and bug10169_025_v2 (for the merge and the
 tests)

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