[posix meterpreter] Various fixes for pre-NPTL threads and zombie reaper #792

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@mephos
Contributor
mephos commented Sep 16, 2012

various fixes for the pre-NPTL zombie reaper :

create it before the scheduler and destroy it after
every zombie, including the zombie reaper itself and the scheduler thread zombie, should be reaped and not left on the system

@brandonprry brandonprry commented on the diff Nov 9, 2012
...urce/meterpreter/source/common/arch/posix/scheduler.c
@@ -50,6 +52,9 @@ DWORD scheduler_destroy( VOID )
thread_destroy(scheduler_thread);
scheduler_thread = NULL;
+ dprintf("stopping zombie reaper");
@brandonprry
brandonprry Nov 9, 2012 Contributor

debug line?

@mephos
mephos Nov 9, 2012 Contributor

dprintf prints debug to log file (/tmp/meterpreter.log.PID) only if debugging is enabled on meterpreter payload (which isn't by default)
it's invaluable when developping and there are this kind of debugging statements everywhere in posix meterpreter code

@brandonprry brandonprry commented on the diff Nov 9, 2012
...urce/meterpreter/source/common/arch/posix/scheduler.c
@@ -50,6 +52,9 @@ DWORD scheduler_destroy( VOID )
thread_destroy(scheduler_thread);
scheduler_thread = NULL;
+ dprintf("stopping zombie reaper");
+ stop_zombie_reaper();
+
dprintf("thread joined .. going for polltable");
@brandonprry
brandonprry Nov 9, 2012 Contributor

debug line?

@brandonprry brandonprry commented on the diff Nov 9, 2012
...urce/meterpreter/source/common/arch/posix/scheduler.c
@@ -81,6 +86,68 @@ DWORD scheduler_destroy( VOID )
return ERROR_SUCCESS;
}
+
+/*
+ * Reap child zombie threads on linux 2.4 (before NPTL)
+ * each thread appears as a process and pthread_join don't necessarily reap it
+ * threads are created using the clone syscall, so use special __WCLONE flag in waitpid
+ */
+
+VOID reap_zombie_thread(void * param)
+{
+ pid_t pid;
+ BOOL success;
+ pthread_internal_t * ptr = (pthread_internal_t *)reaper_tid;
+ // dprintf("reap_zombie_thread : getpid : %d , getppid : %d, gettid : %d, kernel_id : %d, is_kernel_24 : %d", getpid(), getppid(), gettid(), ptr->kernel_id, is_kernel_24);
@brandonprry
brandonprry Nov 9, 2012 Contributor

should we remove old debug lines?

@mephos
mephos Nov 9, 2012 Contributor

when debugging problems, one just have to uncomment the line, recompile and test. saves times when developping for posix meterpreter

@brandonprry brandonprry commented on the diff Nov 9, 2012
...urce/meterpreter/source/common/arch/posix/scheduler.c
+// dprintf("tried to remove pid : %d , success : %d",pid, success);
+ }
+ }
+ }
+ pthread_exit(0);
+}
+
+/*
+ * ask the zombie reaper to stop
+ * before stopping, the reaper will reap all remaining threads created, including the scheduler
+ */
+
+VOID stop_zombie_reaper( VOID )
+{
+ pid_t pid;
+ pthread_internal_t * ptr;
@brandonprry
brandonprry Nov 9, 2012 Contributor

Not sure what the preferences are, but I am not sure of anyone that puts a space before and after the asterisk here.

@mephos
mephos Nov 9, 2012 Contributor

this is the same comment pattern as previous comments in the file and in some other posix meterpreter files

@brandonprry brandonprry commented on the diff Nov 9, 2012
...urce/meterpreter/source/common/arch/posix/scheduler.c
+ }
+ }
+ pthread_exit(0);
+}
+
+/*
+ * ask the zombie reaper to stop
+ * before stopping, the reaper will reap all remaining threads created, including the scheduler
+ */
+
+VOID stop_zombie_reaper( VOID )
+{
+ pid_t pid;
+ pthread_internal_t * ptr;
+ // 2.6/3.x kernel don't have zombie reaper
+ if (is_kernel_24 == 0)
@brandonprry
brandonprry Nov 9, 2012 Contributor

if (!is_kernel_24)?

@brandonprry brandonprry commented on the diff Nov 9, 2012
...urce/meterpreter/source/common/arch/posix/scheduler.c
+ * before stopping, the reaper will reap all remaining threads created, including the scheduler
+ */
+
+VOID stop_zombie_reaper( VOID )
+{
+ pid_t pid;
+ pthread_internal_t * ptr;
+ // 2.6/3.x kernel don't have zombie reaper
+ if (is_kernel_24 == 0)
+ return;
+ // stop zombie reaper
+ stop_reaper = 1;
+ ptr = (pthread_internal_t *)reaper_tid;
+ // reap reaper thread itself
+ pid = waitpid(ptr->kernel_id, NULL, __WCLONE);
+ dprintf("zombie_reaper kernel_id : %d, ret pid : %d (should be equal)",ptr->kernel_id, pid);
@brandonprry
brandonprry Nov 9, 2012 Contributor

debug line

@brandonprry brandonprry commented on the diff Nov 9, 2012
external/source/meterpreter/source/common/base.c
}
list_add( commandThreadList, thread );
-
+/*
+#ifndef _WIN32
+ pthread_internal_t * ptr = (pthread_internal_t *)(thread->pid);
+ dprintf("getpid : %d , getppid : %d, gettid : %d, kernel_id : %d", getpid(), getppid(), gettid(), ptr->kernel_id);
@brandonprry
brandonprry Nov 9, 2012 Contributor

debug line

@brandonprry brandonprry commented on the diff Nov 9, 2012
external/source/meterpreter/source/common/thread.c
@@ -390,6 +409,18 @@ THREAD * thread_create( THREADFUNK funk, LPVOID param1, LPVOID param2 )
return NULL;
}
// __paused_thread free's the allocated memory.
+ if (is_kernel_24 == 1) {
+ if( commandThreadListPID == NULL )
+ {
+ commandThreadListPID = list_create();
+ if( commandThreadListPID == NULL )
+ return NULL;
+ }
+ pthread_internal_t * ptr = (pthread_internal_t *)(thread->pid);
+// dprintf("list_add ptr->kernel_id : %d",ptr->kernel_id);
@brandonprry
brandonprry Nov 9, 2012 Contributor

remove old debug lines?

@jlee-r7 jlee-r7 was assigned Mar 15, 2013
@wvu-r7
Contributor
wvu-r7 commented Jun 11, 2013

Holy crap. This one is old. @mephos???

@mephos
Contributor
mephos commented Jun 14, 2013

The commit should still be good (comments were only about debugging statements present only when debugging is enabled which isn't by default). Unfortunately, I don't have much time to retest it. It's up to you now :)

@wvu-r7
Contributor
wvu-r7 commented Jun 15, 2013

I'll see if I can test this. If there are non-trivial issues, we may need to close this for a while. Too many PRs, and we just don't have time to get through all of them. :(

@wvu-r7
Contributor
wvu-r7 commented Jun 25, 2013

Might want to resubmit this over at the Meterpreter repo.

@wvu-r7 wvu-r7 closed this Jun 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment