On Mon, Dec 17, 2012 at 04:16:36PM +0000, Stuart Henderson wrote:
> I don't know how to fix it but the way ettercap is doing mutex locking
> in ec_thread_new() triggers strict mutex checking code in rthreads.
>
> #0 0x00001b5ca5729d4a in kill () at <stdin>:2
> #1 0x00001b5ca57904aa in abort () at /usr/src/lib/libc/stdlib/abort.c:70
> #2 0x00001b5ca6118505 in _rthread_mutex_lock (mutexp=0x1b5a9ad99320,
> trywait=0, abstime=0x0)
> at /usr/src/lib/librthread/rthread_sync.c:127
> #3 0x00001b5a9ab3a4ce in ec_thread_new (name=0x1b5a9ac8502e "top_half",
> desc=0x1b5a9ac8501b "dispatching module", function=0x1b5a9ab1d73c
> <top_half>, args=0x0) at ec_threads.c:210
> #4 0x00001b5a9ab27455 in main (argc=2, argv=0x7f7ffffd5fe0) at ec_main.c:169
>
> ettercap ec_threads.c:
>
> 185 * creates a new thread on the given function
> 186 */
> 187
> 188 pthread_t ec_thread_new(char *name, char *desc, void *(*function)(void
> *), v oid *args)
> 189 {
> 190 pthread_t id;
> 191
> 192 DEBUG_MSG("ec_thread_new -- %s", name);
> 193
> 194 /*
> 195 * lock the mutex to syncronize with the new thread.
> 196 * the newly created thread will perform INIT_UNLOCK
> 197 * so at the end of this function we are sure that the
> 198 * thread had be initialized
> 199 */
> 200 INIT_LOCK;
> 201
> 202 if (pthread_create(&id, NULL, function, args) != 0)
> 203 ERROR_MSG("not enough resources to create a new thread in this
> process ");
> 204
> 205 ec_thread_register(id, name, desc);
> 206
> 207 DEBUG_MSG("ec_thread_new -- %lu created ", PTHREAD_ID(id));
> 208
> 209 /* the new thread will unlock this */
> 210 INIT_LOCK;
> 211 INIT_UNLOCK;
> 212
> 213 return id;
> 214 }
IMHO, this is crap code. Note that INIT_LOCK and INIT_UNLOCK are
just macros basically expanding to pthread_mutex_lock and
pthread_mutex_unlock on a certain mutex (init_mtx). The comment
says that the newly created thread must do the unlock, but that
can't work (only the thread holding the lock can unlock a mutex).
I really doubt that this works as intended on any OS.
And the INIT_LOCK; INIT_UNLOCK at the end of the function looks
totally bogus to me (and wrong, because it would require a mutex
with the PTHREAD_MUTEX_RECURSIVE attribute).
To add some more fun, the code doesn't check the return values of
pthread_mutex_lock and pthread_mutex_unlock, which may be an
explanation why it *appears* to work on certain other OSs with less
strict default mutexes.
I had a look at the latest available version of ettercap (7.4.1),
but unfortunately there are no relevant changes in this area.
It would be nice if someone with a Linux machine around (and enough
spare time) could try changing the INIT_LOCK and INIT_UNLOCK macros
to error out on return values != 0 and check wether it still works
(it shouldn't -- it should fail).
Ciao,
Kili
