Opened 5 years ago

Closed 5 years ago

#11278 closed defect (fixed)

memory leak in channel_mark_circid_unusable()

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: 025-triaged
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Running moria1 under valgrind:

==29654== 40 bytes in 1 blocks are definitely lost in loss record 10 of 21
==29654==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==29654==    by 0x501A97: tor_malloc_ (util.c:162)
==29654==    by 0x501F65: tor_malloc_zero_ (util.c:188)
==29654==    by 0x4910E1: channel_mark_circid_unusable (circuitlist.c:233)
==29654==    by 0x4822E0: channel_send_destroy (channel.c:2691)
==29654==    by 0x49E219: command_process_create_cell (command.c:234)
==29654==    by 0x49EB92: command_process_cell (command.c:165)
==29654==    by 0x4884FA: channel_tls_handle_cell (channeltls.c:1031)
==29654==    by 0x4BDA68: connection_or_process_cells_from_inbuf (connection_or.c:2059)
==29654==    by 0x4B38E7: connection_handle_read (connection.c:3171)
==29654==    by 0x42E975: conn_read_callback (main.c:735)
==29654==    by 0x52C9343: event_base_loop (in /usr/lib/libevent-1.4.so.2.1.3)
==29654== 40 bytes in 1 blocks are definitely lost in loss record 11 of 21
==29654==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==29654==    by 0x501A97: tor_malloc_ (util.c:162)
==29654==    by 0x501F65: tor_malloc_zero_ (util.c:188)
==29654==    by 0x4910E1: channel_mark_circid_unusable (circuitlist.c:233)
==29654==    by 0x491D4F: circuit_set_p_circid_chan (circuitlist.c:324)
==29654==    by 0x491EA7: circuit_free (circuitlist.c:766)
==29654==    by 0x49222D: circuit_close_all_marked (circuitlist.c:433)
==29654==    by 0x42F3BC: second_elapsed_callback (main.c:1532)
==29654==    by 0x52C9343: event_base_loop (in /usr/lib/libevent-1.4.so.2.1.3)
==29654==    by 0x42C2B0: do_main_loop (main.c:2009)
==29654==    by 0x42D034: tor_main (main.c:2882)
==29654==    by 0x5EFEC8C: (below main) (libc-start.c:228)

Child Tickets

Change History (6)

comment:1 Changed 5 years ago by arma

Does the

  HT_CLEAR(chan_circid_map, &chan_circid_map);

in circuit_free_all() drop them all without freeing them?

comment:2 Changed 5 years ago by nickm

Status: newneeds_review

Possible bugfix in branch "bug11278"

comment:3 Changed 5 years ago by nickm

Keywords: 025-triaged added

comment:4 Changed 5 years ago by andrea

I think this definitely fixes the memory leak. Have you tested it? The assert *should* be okay since the circuit would have already gotten a circuit_set_n_circid_chan(circ, 0, NULL); in circuit_free(), but it'd suck if there turned out to be some other silent bug that this turned into an assert.

comment:5 Changed 5 years ago by nickm

if it's an assertion failure, it's an assert-on-exit, so it's not the end of the universe. I tried it out and didn't get an assert, but if there's a silent bug like you described then it might manifest only under weird conditions. (Maybe in that case an assert-on-exit could be an actual good idea to keep the bug from going unnoticed.)

comment:6 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Andrea says "fine to merge" so merging. Thanks!

Note: See TracTickets for help on using tickets.