[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-bugs] #15245 [Tor]: SETCONF Log results in segfault when not running a relay
#15245: SETCONF Log results in segfault when not running a relay
--------------------+---------------------------------
Reporter: anonym | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: Tor | Version: Tor: 0.2.6.4-rc
Keywords: | Actual Points:
Parent ID: | Points:
--------------------+---------------------------------
To be precise, we get a segfault when `SETCONF`:ing `Log` so its value
changes in any way (but only when no running as a relay). Here's a
backtrace:
{{{
#0 crash_handler (sig=11, si=0xbfffeb4c, ctx_=0xbfffebcc)
at ../src/common/backtrace.c:121
#1 <signal handler called>
#2 0xb7bb9e8d in pthread_mutex_lock ()
from /lib/i386-linux-gnu/libpthread.so.0
#3 0xb7cadd76 in pthread_mutex_lock () from /lib/i386-linux-gnu/libc.so.6
#4 0x8014323c in tor_mutex_acquire (m=m`entry=0x50)
at ../src/common/compat_pthreads.c:125
#5 0x80142175 in threadpool_queue_update (pool=0x0,
dup_fn=dup_fn`entry=0x800f1e50 <worker_state_new>,
fn=fn`entry=0x800f1de0 <update_state_threadfn>,
free_fn=free_fn`entry=0x800f1d80 <worker_state_free>,
arg=arg`entry=0x0)
at ../src/common/workqueue.c:329
#6 0x800f247f in cpuworkers_rotate_keyinfo () at
../src/or/cpuworker.c:181
#7 0x800c9431 in options_act (old_options=0x801ebf48)
at ../src/or/config.c:1731
#8 set_options (new_val=new_val`entry=0x80a52ee8,
msg=msg`entry=0xbffff174)
at ../src/or/config.c:653
#9 0x800cb4e7 in options_trial_assign (list=0x80a503c0,
use_defaults=use_defaults`entry=0, clear_first=clear_first`entry=1,
msg=msg`entry=0xbffff174) at ../src/or/config.c:2066
#10 0x800e98f8 in control_setconf_helper (conn=conn`entry=0x80a51978,
len=len`entry=5, body=<optimized out>, body`entry=0x80a51ae0
"Log\r\n",
use_defaults=0) at ../src/or/control.c:739
#11 0x800ee215 in handle_control_resetconf (body=<optimized out>,
len=<optimized out>, conn=<optimized out>) at ../src/or/control.c:786
#12 connection_control_process_inbuf (conn=conn`entry=0x80a51978)
at ../src/or/control.c:3444
#13 0x800cfb0c in connection_process_inbuf (conn=conn`entry=0x80a51978,
package_partial=package_partial`entry=1) at
../src/or/connection.c:4584
#14 0x800d5cfb in connection_handle_read_impl (conn=0x80a51978)
at ../src/or/connection.c:3340
#15 connection_handle_read (conn=conn`entry=0x80a51978)
at ../src/or/connection.c:3381
#16 0x8002d5e1 in conn_read_callback (fd=21, event=2, _conn=0x80a51978)
at ../src/or/main.c:777
#17 0xb7f4f522 in event_base_loop ()
from /usr/lib/i386-linux-gnu/libevent-2.0.so.5
#18 0x8002dfbf in do_main_loop () at ../src/or/main.c:2104
#19 0x80031955 in tor_main (argc=argc`entry=3, argv=argv`entry=0xbffff724)
at ../src/or/main.c:3078
#20 0x8002a1e3 in main (argc=3, argv=0xbffff724) at
../src/or/tor_main.c:30
}}}
Note that in #5, `pool` is `NULL`, and then we have
`tor_mutex_acquire(&pool->lock);` where `&pool->lock` becomes the pointer
`0x50`, that we later use as an argument in `pthread_mutex_lock()` which
of course leads to this segfault. That `pool` is `NULL` is because when we
call `cpuworkers_rotate_keyinfo()` earlier in the stack, and the global
variable `threadpool` is `NULL` (from initialization) because `cpu_init()`
was never called. I set a breakpoint to verify, but from reading the code
around all calls of `cpu_init()` it's clear it will only be called when
the Tor client is acting as a relay since it's always wrapped inside a `if
(server_mode(...))`.
So, in `options_act()`, we'll end up call `cpuworkers_rotate_keyinfo()`
because `transition_affects_workers` is set, which is done like this:
{{{
const int transition_affects_workers =
old_options && options_transition_affects_workers(old_options,
options);
}}}
Changing the `Log` option is something that probably rightfully will cause
`options_transition_affects_workers` to return true, resulting in this
variable being set, and hence doing all sorts of cpuworker-related stuff
even though they aren't used since we're not running a relay. At least in
the context where we end up calling `cpuworkers_rotate_keyinfo()`, this
seems wrong, but if it's also wrong in the other places when code is run
because `transition_affects_workers`, then it feels like we should instead
do:
{{{
const int transition_affects_workers =
old_options && server_mode(options) &&
options_transition_affects_workers(old_options, options);
}}}
Or something similar. My point is that we need to be running a relay for
the concept of cpuworkers (and options transitions affecting them) to be
relevant. Otherwise a `server_mode()` check is needed somewhere around
that `cpuworkers_rotate_keyinfo()` call, and possibly at other places.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/15245>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs