[FIX] race condition when use sspt_proc

Change-Id: I74f18ff7631c521dceebf137c19d5269c2e2339c
Signed-off-by: Vyacheslav Cherkashin <v.cherkashin@samsung.com>

diff --git a/preload/preload_module.c b/preload/preload_module.c
index 650e7bb..b614042 100644 (file)
--- a/preload/preload_module.c
+++ b/preload/preload_module.c
@@ -266,9 +266,11 @@ static bool __is_proc_mmap_mappable(struct task_struct *task)
                return false;
 
        r_debug_addr += r_state_offset;
-       proc = sspt_proc_by_task(task);
-       if (proc)
+       proc = sspt_proc_get_by_task(task);
+       if (proc) {
                proc->r_state_addr = r_debug_addr;
+               sspt_proc_put(proc);
+       }
 
        if (get_user(state, (unsigned long *)r_debug_addr))
                return false;

diff --git a/us_manager/helper.c b/us_manager/helper.c
index 88749e2..1bdfecf 100644 (file)
--- a/us_manager/helper.c
+++ b/us_manager/helper.c
@@ -69,11 +69,20 @@ static int entry_handler_pf(struct kretprobe_instance *ri, struct pt_regs *regs)
 #endif /* CONFIG_arch */
 
        if (data->addr) {
-               struct sspt_proc *proc = sspt_proc_by_task(current);
+               int ret = 0;
+               struct sspt_proc *proc;
+
+               proc = sspt_proc_get_by_task(current);
+               if (proc) {
+                       if (proc->r_state_addr == data->addr) {
+                               /* skip ret_handler_pf() for current task */
+                               ret = 1;
+                       }
+
+                       sspt_proc_put(proc);
+               }
 
-               if (proc && (proc->r_state_addr == data->addr))
-                       /* skip ret_handler_pf() for current task */
-                       return 1;
+               return ret;
        }
 
        return 0;
@@ -165,7 +174,7 @@ static unsigned long cb_clean_child(void *data)
        struct task_struct *parent = current;
        struct sspt_proc *proc;
 
-       proc = sspt_proc_by_task(parent);
+       proc = sspt_proc_get_by_task(parent);
        if (proc) {
                struct task_struct *child = *(struct task_struct **)data;
 
@@ -174,6 +183,8 @@ static unsigned long cb_clean_child(void *data)
 
                /* disarm urp for child */
                swap_uretprobe_free_task(parent, child, false);
+
+               sspt_proc_put(proc);
        }
 
        atomic_dec(&rm_uprobes_child_cnt);
@@ -460,7 +471,7 @@ static void remove_unmap_probes(struct task_struct *task,
 
        sspt_proc_write_lock();
 
-       proc = sspt_proc_by_task(task);
+       proc = sspt_proc_get_by_task(task);
        if (proc) {
                struct msg_unmap_data msg_data = {
                        .start = umd->start,
@@ -471,6 +482,8 @@ static void remove_unmap_probes(struct task_struct *task,
 
                /* send unmap region */
                sspt_proc_on_each_filter(proc, msg_unmap, (void *)&msg_data);
+
+               sspt_proc_put(proc);
        }
 
        sspt_proc_write_unlock();
@@ -572,7 +585,7 @@ static int ret_handler_mmap(struct kretprobe_instance *ri,
        if (IS_ERR_VALUE(start_addr))
                return 0;
 
-       proc = sspt_proc_by_task(task);
+       proc = sspt_proc_get_by_task(task);
        if (proc == NULL)
                return 0;
 
@@ -580,6 +593,7 @@ static int ret_handler_mmap(struct kretprobe_instance *ri,
        if (vma && check_vma(vma))
                sspt_proc_on_each_filter(proc, msg_map, (void *)vma);
 
+       sspt_proc_put(proc);
        return 0;
 }
 

diff --git a/us_manager/pf/pf_group.c b/us_manager/pf/pf_group.c
index 3665b09..a8fad77 100644 (file)
--- a/us_manager/pf/pf_group.c
+++ b/us_manager/pf/pf_group.c
@@ -512,7 +512,7 @@ static enum pf_inst_flag pfg_check_task(struct task_struct *task)
                        continue;
 
                if (proc == NULL)
-                       proc = sspt_proc_by_task(task);
+                       proc = sspt_proc_get_by_task(task);
 
                if (proc) {
                        flag = flag == PIF_NONE ? PIF_SECOND : flag;
@@ -534,6 +534,7 @@ static enum pf_inst_flag pfg_check_task(struct task_struct *task)
                                        flag = flag == PIF_FIRST ? flag : PIF_ADD_PFG;
                        }
                        mutex_unlock(&proc->filters.mtx);
+                       sspt_proc_put(proc);
                }
        }
        pfg_list_runlock();
@@ -566,9 +567,11 @@ void check_task_and_install(struct task_struct *task)
        switch (flag) {
        case PIF_FIRST:
        case PIF_ADD_PFG:
-               proc = sspt_proc_by_task(task);
-               if (proc)
+               proc = sspt_proc_get_by_task(task);
+               if (proc) {
                        first_install(task, proc);
+                       sspt_proc_put(proc);
+               }
                break;
 
        case PIF_NONE:
@@ -593,15 +596,19 @@ void call_page_fault(struct task_struct *task, unsigned long page_addr)
        switch (flag) {
        case PIF_FIRST:
        case PIF_ADD_PFG:
-               proc = sspt_proc_by_task(task);
-               if (proc)
+               proc = sspt_proc_get_by_task(task);
+               if (proc) {
                        first_install(task, proc);
+                       sspt_proc_put(proc);
+               }
                break;
 
        case PIF_SECOND:
-               proc = sspt_proc_by_task(task);
-               if (proc)
+               proc = sspt_proc_get_by_task(task);
+               if (proc) {
                        subsequent_install(task, proc, page_addr);
+                       sspt_proc_put(proc);
+               }
                break;
 
        case PIF_NONE:
@@ -661,12 +668,13 @@ void call_mm_release(struct task_struct *task)
 {
        struct sspt_proc *proc;
 
-       proc = sspt_proc_by_task(task);
+       proc = sspt_proc_get_by_task(task);
        if (proc) {
                if (task->flags & PF_EXITING)
                        mmr_from_exit(proc);
                else
                        mmr_from_exec(proc);
+               sspt_proc_put(proc);
        }
 }
 

diff --git a/us_manager/sspt/sspt_proc.c b/us_manager/sspt/sspt_proc.c
index 0f70788..28239ac 100644 (file)
--- a/us_manager/sspt/sspt_proc.c
+++ b/us_manager/sspt/sspt_proc.c
@@ -83,30 +83,35 @@ void sspt_proc_write_unlock(void)
        write_unlock(&sspt_proc_rwlock);
 }
 
+struct ktd_proc {
+       struct sspt_proc *proc;
+       spinlock_t lock;
+};
 
 static void ktd_init(struct task_struct *task, void *data)
 {
-       struct sspt_proc **pproc = (struct sspt_proc **)data;
+       struct ktd_proc *kproc = (struct ktd_proc *)data;
 
-       *pproc = NULL;
+       kproc->proc = NULL;
+       spin_lock_init(&kproc->lock);
 }
 
 static void ktd_exit(struct task_struct *task, void *data)
 {
-       struct sspt_proc **pproc = (struct sspt_proc **)data;
+       struct ktd_proc *kproc = (struct ktd_proc *)data;
 
-       WARN_ON(*pproc);
+       WARN_ON(kproc->proc);
 }
 
 struct ktask_data ktd = {
        .init = ktd_init,
        .exit = ktd_exit,
-       .size = sizeof(struct sspt_proc *),
+       .size = sizeof(struct ktd_proc),
 };
 
-static struct sspt_proc **pproc_by_task(struct task_struct *task)
+static struct ktd_proc *kproc_by_task(struct task_struct *task)
 {
-       return (struct sspt_proc **)swap_ktd(&ktd, task);
+       return (struct ktd_proc *)swap_ktd(&ktd, task);
 }
 
 int sspt_proc_init(void)
@@ -121,33 +126,40 @@ void sspt_proc_uninit(void)
 
 void sspt_change_leader(struct task_struct *prev, struct task_struct *next)
 {
-       struct sspt_proc **prev_pproc;
+       struct ktd_proc *prev_kproc;
 
-       prev_pproc = pproc_by_task(prev);
-       if (*prev_pproc) {
-               struct sspt_proc **next_pproc;
+       prev_kproc = kproc_by_task(prev);
+       spin_lock(&prev_kproc->lock);
+       if (prev_kproc->proc) {
+               struct ktd_proc *next_kproc;
 
-               next_pproc = pproc_by_task(next);
+               next_kproc = kproc_by_task(next);
                get_task_struct(next);
 
                /* Change the keeper sspt_proc */
-               BUG_ON(*next_pproc);
-               *next_pproc = *prev_pproc;
-               *prev_pproc = NULL;
+               BUG_ON(next_kproc->proc);
+
+               spin_lock(&next_kproc->lock);
+               next_kproc->proc = prev_kproc->proc;
+               prev_kproc->proc = NULL;
+               spin_unlock(&next_kproc->lock);
 
                /* Set new the task leader to sspt_proc */
-               (*next_pproc)->leader = next;
+               next_kproc->proc->leader = next;
 
                put_task_struct(prev);
        }
+       spin_unlock(&prev_kproc->lock);
 }
 
 void sspt_reset_proc(struct task_struct *task)
 {
-       struct sspt_proc **pproc;
+       struct ktd_proc *kproc;
 
-       pproc = pproc_by_task(task->group_leader);
-       *pproc = NULL;
+       kproc = kproc_by_task(task->group_leader);
+       spin_lock(&kproc->lock);
+       kproc->proc = NULL;
+       spin_unlock(&kproc->lock);
 }
 
 
@@ -232,13 +244,29 @@ void sspt_proc_put(struct sspt_proc *proc)
                kfree(proc);
        }
 }
+EXPORT_SYMBOL_GPL(sspt_proc_put);
 
 struct sspt_proc *sspt_proc_by_task(struct task_struct *task)
 {
-       return *pproc_by_task(task->group_leader);
+       return kproc_by_task(task->group_leader)->proc;
 }
 EXPORT_SYMBOL_GPL(sspt_proc_by_task);
 
+struct sspt_proc *sspt_proc_get_by_task(struct task_struct *task)
+{
+       struct ktd_proc *kproc = kproc_by_task(task->group_leader);
+       struct sspt_proc *proc;
+
+       spin_lock(&kproc->lock);
+       proc = kproc->proc;
+       if (proc)
+               sspt_proc_get(proc);
+       spin_unlock(&kproc->lock);
+
+       return proc;
+}
+EXPORT_SYMBOL_GPL(sspt_proc_get_by_task);
+
 /**
  * @brief Call func() on each proc (no lock)
  *
@@ -280,43 +308,43 @@ EXPORT_SYMBOL_GPL(on_each_proc);
 struct sspt_proc *sspt_proc_get_by_task_or_new(struct task_struct *task)
 {
        static DEFINE_MUTEX(local_mutex);
-       struct sspt_proc **pproc;
+       struct ktd_proc *kproc;
+       struct sspt_proc *proc;
        struct task_struct *leader = task->group_leader;
 
-       pproc = pproc_by_task(leader);
-       if (*pproc)
+       kproc = kproc_by_task(leader);
+       if (kproc->proc)
                goto out;
 
-       /* This lock for synchronizing to create sspt_proc */
-       mutex_lock(&local_mutex);
-       pproc = pproc_by_task(leader);
-       if (*pproc == NULL) {
-               *pproc = sspt_proc_create(leader);
-               if (*pproc) {
-                       sspt_proc_write_lock();
-                       list_add(&(*pproc)->list, &proc_probes_list);
-                       sspt_proc_write_unlock();
-               }
+       proc = sspt_proc_create(leader);
+
+       spin_lock(&kproc->lock);
+       if (kproc->proc == NULL) {
+               sspt_proc_get(proc);
+               kproc->proc = proc;
+               proc = NULL;
+
+               sspt_proc_write_lock();
+               list_add(&kproc->proc->list, &proc_probes_list);
+               sspt_proc_write_unlock();
        }
-       mutex_unlock(&local_mutex);
+       spin_unlock(&kproc->lock);
+
+       if (proc)
+               sspt_proc_cleanup(proc);
 
 out:
-       return *pproc;
+       return kproc->proc;
 }
 
 /**
- * @brief Free all sspt_proc
+ * @brief Check sspt_proc on empty
  *
  * @return Pointer on the sspt_proc struct
  */
-void sspt_proc_free_all(void)
+void sspt_proc_check_empty(void)
 {
-       struct sspt_proc *proc, *n;
-
-       list_for_each_entry_safe(proc, n, &proc_probes_list, list) {
-               list_del(&proc->list);
-               sspt_proc_cleanup(proc);
-       }
+       WARN_ON(!list_empty(&proc_probes_list));
 }
 
 static void sspt_proc_add_file(struct sspt_proc *proc, struct sspt_file *file)

diff --git a/us_manager/sspt/sspt_proc.h b/us_manager/sspt/sspt_proc.h
index 54eb80d..835b9cb 100644 (file)
--- a/us_manager/sspt/sspt_proc.h
+++ b/us_manager/sspt/sspt_proc.h
@@ -84,17 +84,18 @@ struct sspt_proc_cb {
 
 struct list_head *sspt_proc_list(void);
 
-void sspt_proc_cleanup(struct sspt_proc *proc);
+struct sspt_proc *sspt_proc_by_task(struct task_struct *task);
+struct sspt_proc *sspt_proc_get_by_task(struct task_struct *task);
+struct sspt_proc *sspt_proc_get_by_task_or_new(struct task_struct *task);
 struct sspt_proc *sspt_proc_get(struct sspt_proc *proc);
 void sspt_proc_put(struct sspt_proc *proc);
+void sspt_proc_cleanup(struct sspt_proc *proc);
 
 void on_each_proc_no_lock(void (*func)(struct sspt_proc *, void *),
                          void *data);
 void on_each_proc(void (*func)(struct sspt_proc *, void *), void *data);
 
-struct sspt_proc *sspt_proc_by_task(struct task_struct *task);
-struct sspt_proc *sspt_proc_get_by_task_or_new(struct task_struct *task);
-void sspt_proc_free_all(void);
+void sspt_proc_check_empty(void);
 
 struct sspt_file *sspt_proc_find_file(struct sspt_proc *proc,
                                      struct dentry *dentry);

diff --git a/us_manager/us_manager.c b/us_manager/us_manager.c
index 9b66b6e..8ea1041 100644 (file)
--- a/us_manager/us_manager.c
+++ b/us_manager/us_manager.c
@@ -60,7 +60,7 @@ static void do_usm_stop(void)
 
        uninstall_all();
        unregister_helper_bottom();
-       sspt_proc_free_all();
+       sspt_proc_check_empty();
 }
 
 static int do_usm_start(void)