Opened 4 years ago

Closed 4 years ago

#18673 closed defect (fixed)

Memory leak with TestingEnableCellStatsEvent

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points: small
Reviewer: Sponsor: SponsorS-can

Description

Running chutney with asan, I see:

=================================================================
==13895==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1040 byte(s) in 65 object(s) allocated from:
    #0 0x55db7087dacb in __interceptor_malloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
    #1 0x55db70d23dd2 in tor_malloc_ /home/nickm/src/tor/src/common/util.c:171:12
    #2 0x55db70cf4847 in smartlist_new /home/nickm/src/tor/src/common/container.c:34:21
    #3 0x55db7093e34d in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2652:38
    #4 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
    #5 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
    #6 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
    #7 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)

Indirect leak of 37376 byte(s) in 23 object(s) allocated from:
    #0 0x55db7087de1e in realloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:61:3
    #1 0x55db70d242dc in tor_realloc_ /home/nickm/src/tor/src/common/util.c:259:12
    #2 0x55db70d24550 in tor_reallocarray_ /home/nickm/src/tor/src/common/util.c:280:10
    #3 0x55db70cf509f in smartlist_ensure_capacity /home/nickm/src/tor/src/common/container.c:87:16
    #4 0x55db70cf4df1 in smartlist_add /home/nickm/src/tor/src/common/container.c:101:3
    #5 0x55db7093e3ac in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2653:9
    #6 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
    #7 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
    #8 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
    #9 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)

Indirect leak of 18636 byte(s) in 4659 object(s) allocated from:
    #0 0x55db7087dacb in __interceptor_malloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
    #1 0x55db70d23dd2 in tor_malloc_ /home/nickm/src/tor/src/common/util.c:171:12
    #2 0x55db70d23f6f in tor_malloc_zero_ /home/nickm/src/tor/src/common/util.c:197:18
    #3 0x55db7093e195 in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2645:11
    #4 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
    #5 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
    #6 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
    #7 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)

Indirect leak of 5376 byte(s) in 42 object(s) allocated from:
    #0 0x55db7087dacb in __interceptor_malloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
    #1 0x55db70d23dd2 in tor_malloc_ /home/nickm/src/tor/src/common/util.c:171:12
    #2 0x55db70d240a4 in tor_malloc_zero_ /home/nickm/src/tor/src/common/util.c:197:18
    #3 0x55db70d240a4 in tor_calloc_ /home/nickm/src/tor/src/common/util.c:235
    #4 0x55db70cf48f7 in smartlist_new /home/nickm/src/tor/src/common/container.c:37:14
    #5 0x55db7093e34d in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2652:38
    #6 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
    #7 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
    #8 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
    #9 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)

SUMMARY: AddressSanitizer: 62428 byte(s) leaked in 4789 allocation(s).

Now, this only seems to happen with TestingEnableCellStatsEvent enabled, so it's not a showstopper on the real network, but it's no fun. We should fix this leak.

Child Tickets

Change History (7)

comment:1 Changed 4 years ago by nickm

Status: newneeds_review

bug18673_028 should fix this.

comment:2 Changed 4 years ago by nickm

Thanks to Sebastian for comments. Now see bug18673_028_v2 for a couple of cosmetic improvements (comments and commit message).

comment:3 Changed 4 years ago by Sebastian

lgtm

comment:4 Changed 4 years ago by nickm

Thanks! I think I'm going to leave it out of the alpha, though. It can go into the rc.

comment:5 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged to 0.2.8 and forward!

comment:6 Changed 4 years ago by yawning

Resolution: fixed
Status: closedreopened

This breaks unit tests, and possibly lots of other things with a null pointer deref.

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 670c5b3..1efb7ef 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -760,7 +760,7 @@ or_circuit_new(circid_t p_circ_id, channel_t *p_chan)
 void
 circuit_clear_testing_cell_stats(circuit_t *circ)
 {
-  if (!circ)
+  if (!circ || !circ->testing_cell_stats)
     return;
   SMARTLIST_FOREACH(circ->testing_cell_stats, testing_cell_stats_entry_t *,
                     ent, tor_free(ent));

comment:7 Changed 4 years ago by nickm

Resolution: fixed
Status: reopenedclosed

d;oh! Merged, pushed.

Note: See TracTickets for help on using tickets.