Opened 9 years ago

Last modified 7 years ago

#1365 closed defect (Fixed)

Repeatedly freeing random heap addresses

Reported by: arma Owned by:
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.2.1.25
Severity: Keywords:
Cc: arma, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

keb got a seg fault running 0.2.2.11-alpha:

(gdb) where
#0 0xb7c79947 in raise () from /lib/tls/libc.so.6
#1 0xb7c7b0c9 in abort () from /lib/tls/libc.so.6
#2 0xb7caf6ba in fsetlocking () from /lib/tls/libc.so.6
#3 0xb7cb6f7f in mallopt () from /lib/tls/libc.so.6
#4 0xb7cb7022 in free () from /lib/tls/libc.so.6
#5 0x080fbb99 in geoip_get_request_history (now=1271699586, action=GEOIP_CLIENT_NETWORKSTATUS_V2)

at geoip.c:953

#6 0x080fd7cf in geoip_dirreq_stats_write (now=1271699586) at geoip.c:1009
#7 0x0805236c in second_elapsed_callback (fd=-1, event=1, args=0x0) at main.c:990
#8 0xb7f1741e in event_base_loop (base=0x8972158, flags=0) at event.c:385
#9 0x08052818 in do_main_loop () at main.c:1512
#10 0x08052a97 in tor_main (argc=3, argv=0xbf980784) at main.c:2137
#11 0x0804d962 in main (argc=Cannot access memory at address 0x447a
) at tor_main.c:30
(gdb)

Here's the stanza in question:

strings = smartlist_create();
SMARTLIST_FOREACH(entries, c_hist_t *, ent, {

char buf[32];
tor_snprintf(buf, sizeof(buf), "%s=%u", ent->country, ent->total);
smartlist_add(strings, buf);

});

result = smartlist_join_strings(strings, ",", 0, NULL);
SMARTLIST_FOREACH(strings, char *, cp, tor_free(cp));

See how we're adding the address of buf over and over to the "strings" smartlist,
even though we keep deallocating it from the heap? And then on the last line, we
try to free each of those deallocated addresses?

Seems we should instead be doing

smartlist_add(strings, tor_strdup(buf));

This bug only triggers when you've turned on the DirReqStatistics config option.

Bug added in git ed174245 (June 18 2008), aka svn r15345, which it looks like went
out as part of 0.2.1.2-alpha. Sounds like a job for Mr Backport too.

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (6)

comment:1 Changed 9 years ago by nickm

Looks like a job for asprintf!

comment:2 Changed 9 years ago by nickm

or rather, it would, if asprintf were backported. okay, will fix the dumb way since I'm still awake.

comment:3 Changed 9 years ago by nickm

Okay. This isn't in ed174245. That version handled stuff just fine. This was caused by 47e919424da, which removed
the strdup without changing the corresponding snprintf to asprintf.

comment:4 Changed 9 years ago by nickm

Should be fixed in f2c30e9

comment:5 Changed 9 years ago by nickm

flyspray2trac: bug closed.

comment:6 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.