From 8d133e88b32ed76cd66c5d7b36cc264d7b55faa8 Mon Sep 17 00:00:00 2001 From: Vignesh Babu Date: Fri, 13 Sep 2019 00:48:34 -0700 Subject: [PATCH] fixed bugs which caused task_struct memory leak in ptrace.c and kernel memory leak in hashmap --- src/core/common.c | 5 +- .../linux-4.4.5/kernel/ptrace.c | 61 ++++++++++++------- src/tracer/Makefile | 5 ++ src/tracer/libperf/config.status | 2 +- src/tracer/libperf/libtool | 2 +- src/tracer/tracer.c | 3 +- src/tracer/utils/Makefile | 7 +++ src/tracer/utils/hashmap.c | 3 +- src/tracer/utils/hashmap_test.c | 30 +++++++++ src/tracer/utils/linked_list_test.c | 30 +++++++++ src/utils/hashmap.c | 3 +- 11 files changed, 121 insertions(+), 30 deletions(-) create mode 100644 src/tracer/utils/Makefile create mode 100644 src/tracer/utils/hashmap_test.c create mode 100644 src/tracer/utils/linked_list_test.c diff --git a/src/core/common.c b/src/core/common.c index 8e36c45..4ddad7e 100644 --- a/src/core/common.c +++ b/src/core/common.c @@ -114,6 +114,7 @@ int pop_schedule_list(tracer * tracer_entry) { if (head != NULL) { pid = head->pid; hmap_remove_abs(&tracer_entry->valid_children, head->pid); + hmap_remove_abs(&tracer_entry->ignored_children, head->pid); kfree(head); return pid; } @@ -334,8 +335,8 @@ void add_to_tracer_schedule_queue(tracer * tracer_entry, sched_setscheduler(tracee, SCHED_RR, &sp); hmap_put_abs(&tracer_entry->valid_children, new_elem->pid, new_elem); PDEBUG_E("Add to tracer schedule queue: " - "Tracer %d, tracee %d. Succeeded.\n", tracer_entry->tracer_id, - tracee->pid); + "Tracer %d, tracee %d. Succeeded. Schedule list size: %d\n", tracer_entry->tracer_task->pid, + tracee->pid, schedule_list_size(tracer_entry)); } diff --git a/src/kernel_changes/linux-4.4.5/kernel/ptrace.c b/src/kernel_changes/linux-4.4.5/kernel/ptrace.c index 2d44199..ea2140e 100755 --- a/src/kernel_changes/linux-4.4.5/kernel/ptrace.c +++ b/src/kernel_changes/linux-4.4.5/kernel/ptrace.c @@ -1102,37 +1102,54 @@ SYSCALL_DEFINE4(ptrace, long, request, long, pid, unsigned long, addr, if (request == PTRACE_GET_REM_MULTISTEP) { unsigned long rem_steps = child->ptrace_msteps; - if (copy_to_user(datalp, &rem_steps, sizeof(unsigned long))) - return -EFAULT; - return 0; - } else if (request == PTRACE_SET_REM_MULTISTEP) { - if (__get_user(n_steps, datalp)) - return -EFAULT; + if (copy_to_user(datalp, &rem_steps, sizeof(unsigned long))) { + ret = -EFAULT; + } else { + ret = 0; + } + goto out_put_task_struct; - child->ptrace_msteps = n_steps; - child->n_ints = 0; - trace_printk("Ptrace: Pid: %d, set rem multistep: ptrace_msteps: %lu\n", child->pid, child->ptrace_msteps); - return 0; + } else if (request == PTRACE_SET_REM_MULTISTEP) { + if (__get_user(n_steps, datalp)) { + ret = -EFAULT; + } else { + child->ptrace_msteps = n_steps; + child->n_ints = 0; + trace_printk("Ptrace: Pid: %d, set rem multistep: ptrace_msteps: %lu\n", child->pid, child->ptrace_msteps); + ret = 0; + } + goto out_put_task_struct; } else if (request == PTRACE_GET_MSTEP_FLAGS) { flags = child->ptrace_mflags; trace_printk("Ptrace: Pid: %d, get ptrace flags: %lX\n", child->pid, child->ptrace_mflags); - if (copy_to_user(datalp, &flags, sizeof(unsigned long))) - return -EFAULT; - return 0; + if (copy_to_user(datalp, &flags, sizeof(unsigned long))) { + ret = -EFAULT; + } else{ + ret = 0; + } + goto out_put_task_struct; + } else if (request == PTRACE_SET_DELTA_BUFFER_WINDOW) { - if (__get_user(n_steps, datalp)) - return -EFAULT; - child->past_physical_time = n_steps; - trace_printk("Ptrace: Pid: %d, set rem delta buffer window size: %lu\n", child->pid, child->past_physical_time); - return 0; + if (__get_user(n_steps, datalp)) { + ret = -EFAULT; + } else { + child->past_physical_time = n_steps; + trace_printk("Ptrace: Pid: %d, set rem delta buffer window size: %lu\n", child->pid, child->past_physical_time); + ret = 0; + } + goto out_put_task_struct; + } else if (request == PTRACE_GET_OVERSHOOT_ERROR) { unsigned long error = child->past_virtual_time; child->past_virtual_time = 0; - if (copy_to_user(datalp, &error, sizeof(unsigned long))) - return -EFAULT; - trace_printk("Ptrace: Pid: %d, overshoot error: %lu\n", child->pid, child->past_virtual_time); - return 0; + if (copy_to_user(datalp, &error, sizeof(unsigned long))) { + ret = -EFAULT; + } else { + trace_printk("Ptrace: Pid: %d, overshoot error: %lu\n", child->pid, child->past_virtual_time); + ret = 0; + } + goto out_put_task_struct; } ret = ptrace_check_attach(child, request == PTRACE_KILL || diff --git a/src/tracer/Makefile b/src/tracer/Makefile index 4f6933d..b7a0992 100644 --- a/src/tracer/Makefile +++ b/src/tracer/Makefile @@ -21,6 +21,7 @@ build_tracer: @mv tracer /usr/bin @mv cmd_sender /usr/bin @cp /usr/local/lib/libperf.so.0 /usr/lib/ + @cd ${tracer_dir}/utils && $(MAKE) build build_tracer_debug: @gcc -w -I/usr/local/include/perf -I/usr/local/lib/perf/include -I${tracer_dir}/utils \ @@ -32,6 +33,7 @@ build_tracer_debug: @mv tracer /usr/bin @mv cmd_sender /usr/bins @cp /usr/local/lib/libperf.so.0 /usr/lib/ + @cd ${tracer_dir}/utils && $(MAKE) build build_tracer_test: @gcc -w -I/usr/local/include/perf -I/usr/local/lib/perf/include -I${tracer_dir}/utils \ @@ -43,6 +45,7 @@ build_tracer_test: @mv tracer_test /usr/bin @mv cmd_sender /usr/bin @cp /usr/local/lib/libperf.so.0 /usr/lib/ + @cd ${tracer_dir}/utils && $(MAKE) build clean: @cd ${tracer_dir}/libperf && ./configure && $(MAKE) clean @@ -51,9 +54,11 @@ clean: @rm -f /usr/bin/tracer > /dev/null @rm -f /usr/bin/tracer_test > /dev/null @rm -f /usr/bin/cmd_sender > /dev/null + @cd ${tracer_dir}/utils && $(MAKE) clean build_tests: @cd ${tracer_dir}/tests && $(MAKE) build + @cd ${tracer_dir}/utils && $(MAKE) build build: libs build_tracer build_tracer_test build_tests diff --git a/src/tracer/libperf/config.status b/src/tracer/libperf/config.status index 7a8eb04..e6a2a8b 100755 --- a/src/tracer/libperf/config.status +++ b/src/tracer/libperf/config.status @@ -680,7 +680,7 @@ finish_cmds='PATH="\$PATH:/sbin" ldconfig -n $libdir' finish_eval='' hardcode_into_libs='yes' sys_lib_search_path_spec='/usr/lib/gcc/x86_64-linux-gnu/6 /usr/lib/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib ' -configure_time_dlsearch_path='/lib /usr/lib /usr/lib/x86_64-linux-gnu/libfakeroot /usr/local/lib /lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu/mesa-egl /usr/lib/x86_64-linux-gnu/mesa ' +configure_time_dlsearch_path='/lib /usr/lib /usr/lib/x86_64-linux-gnu/libfakeroot /usr/local/lib /lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu/mesa-egl /usr/lib/x86_64-linux-gnu/mesa /lib32 /usr/lib32 ' configure_time_lt_sys_library_path='' hardcode_action='immediate' enable_dlopen='unknown' diff --git a/src/tracer/libperf/libtool b/src/tracer/libperf/libtool index 85befdd..ac7328d 100755 --- a/src/tracer/libperf/libtool +++ b/src/tracer/libperf/libtool @@ -285,7 +285,7 @@ hardcode_into_libs=yes sys_lib_search_path_spec="/usr/lib/gcc/x86_64-linux-gnu/6 /usr/lib/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib " # Detected run-time system search path for libraries. -sys_lib_dlsearch_path_spec="/lib /usr/lib /usr/lib/x86_64-linux-gnu/libfakeroot /usr/local/lib /lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu/mesa-egl /usr/lib/x86_64-linux-gnu/mesa " +sys_lib_dlsearch_path_spec="/lib /usr/lib /usr/lib/x86_64-linux-gnu/libfakeroot /usr/local/lib /lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu/mesa-egl /usr/lib/x86_64-linux-gnu/mesa /lib32 /usr/lib32 " # Explicit LT_SYS_LIBRARY_PATH set during ./configure time. configure_time_lt_sys_library_path="" diff --git a/src/tracer/tracer.c b/src/tracer/tracer.c index 0bcc40e..2fedc80 100644 --- a/src/tracer/tracer.c +++ b/src/tracer/tracer.c @@ -19,7 +19,7 @@ #define PTRACE_SET_DELTA_BUFFER_WINDOW 0x42f4 #define TRACER_RESULTS 'J' -#define BUFFER_WINDOW_SIZE 100 +#define BUFFER_WINDOW_SIZE 50 #ifdef TEST @@ -1282,7 +1282,6 @@ int main(int argc, char * argv[]) { if (!hmap_get_abs(&tracees, processes_pids[i])) { ignored_pids[j] = processes_pids[i]; - printf("Ignoring PID: %d\n", ignored_pids[j]); j++; } } diff --git a/src/tracer/utils/Makefile b/src/tracer/utils/Makefile new file mode 100644 index 0000000..18380db --- /dev/null +++ b/src/tracer/utils/Makefile @@ -0,0 +1,7 @@ +build: + @gcc linked_list_test.c linkedlist.c -o linked_list_test + @gcc hashmap_test.c hashmap.c linkedlist.c -o hmap_test + +clean: + @rm linked_list_test + @rm hmap_test diff --git a/src/tracer/utils/hashmap.c b/src/tracer/utils/hashmap.c index d748d45..3a00b9c 100755 --- a/src/tracer/utils/hashmap.c +++ b/src/tracer/utils/hashmap.c @@ -207,7 +207,8 @@ void hmap_remove_abs(hashmap * h, int key) { while (head != NULL) { temp = (hashmap_elem *) head->item; if (temp->key_val == key) { - temp->key_val = 0; + llist_remove_at(list, i); + free(temp); return; } i++; diff --git a/src/tracer/utils/hashmap_test.c b/src/tracer/utils/hashmap_test.c new file mode 100644 index 0000000..57dab34 --- /dev/null +++ b/src/tracer/utils/hashmap_test.c @@ -0,0 +1,30 @@ +#include "hashmap.h" +#include +#include + + +int main (char * argv, int argc) { + hashmap test; + hmap_init(&test, 1000); + int a [100]; + for (int i = 0; i < 100; i++) { + a[i] = i; + hmap_put_abs(&test, i, &a[i]); + } + for (int i = 0; i < 50; i++) { + hmap_put_abs(&test, i, &a[i]); + hmap_remove_abs(&test, i); + } + for (int i = 50; i < 100; i++) { + printf("Hmap get = %d\n", *(int *) hmap_get_abs(&test, i)); + } + + for (int i = 0; i < 50; i++) { + if (hmap_get_abs(&test, i)) + return 0; + } + + hmap_destroy(&test); + + return 0; +} diff --git a/src/tracer/utils/linked_list_test.c b/src/tracer/utils/linked_list_test.c new file mode 100644 index 0000000..70cb49b --- /dev/null +++ b/src/tracer/utils/linked_list_test.c @@ -0,0 +1,30 @@ +#include "linkedlist.h" +#include +#include + + +int main (char * argv, int argc) { + llist test_list; + llist_init(&test_list); + int a [100]; + for (int i = 0; i < 100; i++) { + a[i] = i; + llist_append(&test_list, &a[i]); + } + for (int i = 0; i < 50; i++) { + llist_append(&test_list, &a[i]); + llist_pop(&test_list); + } + + for (int i = 0; i < 50; i++) { + llist_requeue(&test_list); + } + + for (int i = 0; i < 50; i++) { + llist_remove_at(&test_list, 0); + } + printf("LList size = %d", llist_size(&test_list)); + llist_destroy(&test_list); + + return 0; +} diff --git a/src/utils/hashmap.c b/src/utils/hashmap.c index 1f13883..2ef1d2b 100755 --- a/src/utils/hashmap.c +++ b/src/utils/hashmap.c @@ -218,7 +218,8 @@ void hmap_remove_abs(hashmap * h, int key) { while (head != NULL) { temp = (hashmap_elem *) head->item; if (temp->key_val == key) { - temp->key_val = 0; + llist_remove_at(list, i); + kfree(temp); spin_unlock(&h->hmap_lock); return; }