From c10e49a445762009aaed62d834d5f96ea6236973 Mon Sep 17 00:00:00 2001 From: Anatolii Nikulin Date: Thu, 29 Oct 2015 11:59:22 +0300 Subject: [PATCH] [IMPROVE] use list for saving target binary Dynamically changed array is more difficult to implement and may cause memory errors. Using the list instead of an array is preferable. Change-Id: I20b5ce6be161db9b83a89080ff8dc75e99970651 Signed-off-by: Anatolii Nikulin --- preload/preload_control.c | 197 +++++++++++++++++----------------------------- 1 file changed, 71 insertions(+), 126 deletions(-) diff --git a/preload/preload_control.c b/preload/preload_control.c index c731464..08d922e 100644 --- a/preload/preload_control.c +++ b/preload/preload_control.c @@ -1,7 +1,8 @@ #include #include -#include +#include #include +#include #include @@ -11,123 +12,84 @@ #include "preload_probe.h" #include "preload_module.h" - -#define DEFAULT_SLOTS_COUNT 5 -#define DEFAULT_SLOTS_STEP 2 - struct bin_desc { + struct list_head list; struct dentry *dentry; char *filename; }; -static struct bin_desc *target_binaries = NULL; -static unsigned int target_binaries_cnt = 0; -static unsigned int target_binaries_slots = 0; - -static DEFINE_MUTEX(__target_binaries_mutex); - - -static inline void __target_binaries_lock(void) -{ - mutex_lock(&__target_binaries_mutex); -} - -static inline void __target_binaries_unlock(void) -{ - mutex_unlock(&__target_binaries_mutex); -} +static LIST_HEAD(target_binaries_list); +static DECLARE_RWSEM(target_binaries_lock); +static int target_binaries_cnt = 0; static inline struct task_struct *__get_task_struct(void) { return current; } -static int __alloc_target_binaries_no_lock(unsigned int cnt) +static struct bin_desc *__alloc_target_binary(struct dentry *dentry, + char *name, int namelen) { - target_binaries = kmalloc(sizeof(*target_binaries) * cnt, GFP_KERNEL); - if (target_binaries == NULL) - return -ENOMEM; + struct bin_desc *p = NULL; - target_binaries_slots = cnt; + p = kzalloc(sizeof(*p), GFP_KERNEL); + if (!p) + return NULL; - return 0; + INIT_LIST_HEAD(&p->list); + p->filename = kmalloc(namelen + 1, GFP_KERNEL); + if (!p->filename) + goto fail; + memcpy(p->filename, name, namelen); + p->filename[namelen] = '\0'; + p->dentry = dentry; + + return p; +fail: + kfree(p); + return NULL; } -static int __alloc_target_binaries(unsigned int cnt) +static void __free_target_binary(struct bin_desc *p) { - int ret; - - __target_binaries_lock(); - ret = __alloc_target_binaries_no_lock(cnt); - __target_binaries_unlock(); - - return ret; + kfree(p->filename); + kfree(p); } static void __free_target_binaries(void) { - int i; - - __target_binaries_lock(); + struct bin_desc *p, *n; - for (i = 0; i < target_binaries_cnt; i++) { - put_dentry(target_binaries[i].dentry); - kfree(target_binaries[i].filename); + down_write(&target_binaries_lock); + list_for_each_entry_safe(p, n, &target_binaries_list, list) { + list_del(&p->list); + __free_target_binary(p); } - - kfree(target_binaries); target_binaries_cnt = 0; - target_binaries_slots = 0; - - __target_binaries_unlock(); -} - -static int __grow_target_binaries(void) -{ - struct bin_desc *tmp = target_binaries; - int i, ret; - - __target_binaries_lock(); - - ret = __alloc_target_binaries_no_lock(target_binaries_slots + DEFAULT_SLOTS_STEP); - if (ret != 0) - return ret; - - for (i = 0; i < target_binaries_cnt; i++) { - target_binaries[i].dentry = tmp[i].dentry; - target_binaries[i].filename = tmp[i].filename; - } - - __target_binaries_unlock(); - - kfree(tmp); - - return 0; + up_write(&target_binaries_lock); } static bool __check_dentry_already_exist(struct dentry *dentry) { - int i; + struct bin_desc *p; bool ret = false; - __target_binaries_lock(); - - for (i = 0; i < target_binaries_cnt; i++) { - if (target_binaries[i].dentry == dentry) { + down_read(&target_binaries_lock); + list_for_each_entry(p, &target_binaries_list, list) { + if (p->dentry == dentry) { ret = true; - goto check_dentry_unlock; + goto out; } } +out: + up_read(&target_binaries_lock); -check_dentry_unlock: - __target_binaries_unlock(); - - return ret; + return false; } static int __add_target_binary(struct dentry *dentry, char *filename) { - int ret; + struct bin_desc *p; size_t len; if (__check_dentry_already_exist(dentry)) { @@ -135,35 +97,23 @@ static int __add_target_binary(struct dentry *dentry, char *filename) return EALREADY; } - - if (target_binaries_slots == target_binaries_cnt) { - ret = __grow_target_binaries(); - if (ret != 0) - return ret; - } - /* Filename should be < PATH_MAX */ len = strnlen(filename, PATH_MAX); if (len == PATH_MAX) return -EINVAL; - __target_binaries_lock(); - - target_binaries[target_binaries_cnt].dentry = dentry; - target_binaries[target_binaries_cnt].filename = kmalloc(len + 1, GFP_KERNEL); - memcpy(target_binaries[target_binaries_cnt].filename, filename, len + 1); - ++target_binaries_cnt; + p = __alloc_target_binary(dentry, filename, len); + if (!p) + return -ENOMEM; - __target_binaries_unlock(); + down_write(&target_binaries_lock); + list_add_tail(&p->list, &target_binaries_list); + target_binaries_cnt++; + up_write(&target_binaries_lock); return 0; } -static char *__get_binary_name(struct bin_desc *bin) -{ - return bin->filename; -} - static struct dentry *__get_caller_dentry(struct task_struct *task, unsigned long caller) { @@ -186,13 +136,7 @@ get_caller_dentry_fail: static bool __check_if_instrumented(struct task_struct *task, struct dentry *dentry) { - int i; - - for (i = 0; i < target_binaries_cnt; i++) - if (target_binaries[i].dentry == dentry) - return true; - - return false; + return __check_dentry_already_exist(dentry); } static bool __is_instrumented(void *caller) @@ -247,32 +191,35 @@ int preload_control_add_instrumented_binary(char *filename) int preload_control_clean_instrumented_bins(void) { __free_target_binaries(); - return __alloc_target_binaries(DEFAULT_SLOTS_COUNT); + + return 0; } unsigned int preload_control_get_bin_names(char ***filenames_p) { - int i; - unsigned int ret = 0; + unsigned int i, ret = 0; + struct bin_desc *p; + char **a = NULL; + down_read(&target_binaries_lock); if (target_binaries_cnt == 0) - return 0; - - __target_binaries_lock(); - - *filenames_p = kmalloc(sizeof(**filenames_p) * target_binaries_cnt, - GFP_KERNEL); - if (*filenames_p == NULL) - goto get_binaries_names_out; + goto out; - for (i = 0; i < target_binaries_cnt; i++) - (*filenames_p)[i] = __get_binary_name(&target_binaries[i]); + a = kmalloc(sizeof(*a) * target_binaries_cnt, GFP_KERNEL); + if (!a) + goto out; - ret = target_binaries_cnt; - -get_binaries_names_out: - __target_binaries_unlock(); + i = 0; + list_for_each_entry(p, &target_binaries_list, list) { + if (i >= target_binaries_cnt) + break; + a[i++] = p->filename; + } + *filenames_p = a; + ret = i; +out: + up_read(&target_binaries_lock); return ret; } @@ -283,7 +230,7 @@ void preload_control_release_bin_names(char ***filenames_p) int preload_control_init(void) { - return __alloc_target_binaries(DEFAULT_SLOTS_COUNT); + return 0; } void preload_control_exit(void) @@ -291,5 +238,3 @@ void preload_control_exit(void) __free_target_binaries(); } -#undef DEFAULT_SLOTS_STEP -#undef DEFAULT_SLOTS_COUNT -- 2.7.4