fixed bugs which caused task_struct memory leak in ptrace.c and kerne…

…l memory leak in hashmap
Vignesh2208 · Sep 13, 2019 · 8d133e8 · 8d133e8
1 parent a28aa81
commit 8d133e8
Show file tree

Hide file tree

Showing 11 changed files with 121 additions and 30 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,30 @@
+#include "hashmap.h"
+#include <stdio.h>
+#include <stdlib.h>
+
+
+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
@@ -0,0 +1,30 @@
+#include "linkedlist.h"
+#include <stdio.h>
+#include <stdlib.h>
+
+
+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
@@ -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;
 		}