[FIX] Add reference counting to sspt_proc

To avoid usage after deletion bugs.

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

diff --git a/us_manager/pf/pf_group.c b/us_manager/pf/pf_group.c
index 28b232a..898ad7e 100644 (file)
--- a/us_manager/pf/pf_group.c
+++ b/us_manager/pf/pf_group.c
@@ -67,41 +67,19 @@ static struct pl_struct *create_pl_struct(struct sspt_proc *proc)
                return NULL;
 
        INIT_LIST_HEAD(&pls->list);
-       pls->proc = proc;
+       pls->proc = sspt_proc_get(proc);
 
        return pls;
 }
 
 static void free_pl_struct(struct pl_struct *pls)
 {
+       sspt_proc_put(pls->proc);
        kfree(pls);
 }
-
-static void add_pl_struct(struct pf_group *pfg, struct pl_struct *pls)
-{
-       list_add(&pls->list, &pfg->proc_list);
-}
-
-static void del_pl_struct(struct pl_struct *pls)
-{
-       list_del(&pls->list);
-}
-
-static struct pl_struct *find_pl_struct(struct pf_group *pfg,
-                                       struct task_struct *task)
-{
-       struct pl_struct *pls;
-
-       list_for_each_entry(pls, &pfg->proc_list, list) {
-               if (pls->proc->tgid == task->tgid)
-                       return pls;
-       }
-
-       return NULL;
-}
 /* struct pl_struct */
 
-static struct pf_group *create_pfg(void)
+static struct pf_group *pfg_create(void)
 {
        struct pf_group *pfg = kmalloc(sizeof(*pfg), GFP_KERNEL);
 
@@ -128,25 +106,44 @@ create_pfg_fail:
        return NULL;
 }
 
-static void free_pfg(struct pf_group *pfg)
+static void pfg_free(struct pf_group *pfg)
 {
-       struct pl_struct *pl;
+       struct pl_struct *pl, *n;
 
        free_img_proc(pfg->i_proc);
        free_pf(&pfg->filter);
-       list_for_each_entry(pl, &pfg->proc_list, list)
+       list_for_each_entry_safe(pl, n, &pfg->proc_list, list) {
                sspt_proc_del_filter(pl->proc, pfg);
+               free_pl_struct(pl);
+       }
+
        kfree(pfg);
 }
 
+static int pfg_add_proc(struct pf_group *pfg, struct sspt_proc *proc)
+{
+       struct pl_struct *pls;
+
+       pls = create_pl_struct(proc);
+       if (pls == NULL)
+               return -ENOMEM;
+
+       spin_lock(&pfg->pl_lock);
+       list_add(&pls->list, &pfg->proc_list);
+       spin_unlock(&pfg->pl_lock);
+
+       return 0;
+}
+
+
 /* called with pfg_list_lock held */
-static void add_pfg_by_list(struct pf_group *pfg)
+static void pfg_add_to_list(struct pf_group *pfg)
 {
        list_add(&pfg->list, &pfg_list);
 }
 
 /* called with pfg_list_lock held */
-static void del_pfg_by_list(struct pf_group *pfg)
+static void pfg_del_from_list(struct pf_group *pfg)
 {
        list_del(&pfg->list);
 }
@@ -268,13 +265,13 @@ struct pf_group *get_pf_group_by_dentry(struct dentry *dentry, void *priv)
                }
        }
 
-       pfg = create_pfg();
+       pfg = pfg_create();
        if (pfg == NULL)
                goto unlock;
 
        set_pf_by_dentry(&pfg->filter, dentry, priv);
 
-       add_pfg_by_list(pfg);
+       pfg_add_to_list(pfg);
 
 unlock:
        write_unlock(&pfg_list_lock);
@@ -301,13 +298,13 @@ struct pf_group *get_pf_group_by_tgid(pid_t tgid, void *priv)
                }
        }
 
-       pfg = create_pfg();
+       pfg = pfg_create();
        if (pfg == NULL)
                goto unlock;
 
        set_pf_by_tgid(&pfg->filter, tgid, priv);
 
-       add_pfg_by_list(pfg);
+       pfg_add_to_list(pfg);
 
 unlock:
        write_unlock(&pfg_list_lock);
@@ -335,19 +332,19 @@ struct pf_group *get_pf_group_by_comm(char *comm, void *priv)
                }
        }
 
-       pfg = create_pfg();
+       pfg = pfg_create();
        if (pfg == NULL)
                goto unlock;
 
        ret = set_pf_by_comm(&pfg->filter, comm, priv);
        if (ret) {
                printk(KERN_ERR "ERROR: set_pf_by_comm, ret=%d\n", ret);
-               free_pfg(pfg);
+               pfg_free(pfg);
                pfg = NULL;
                goto unlock;
        }
 
-       add_pfg_by_list(pfg);
+       pfg_add_to_list(pfg);
 unlock:
        write_unlock(&pfg_list_lock);
        return pfg;
@@ -372,13 +369,13 @@ struct pf_group *get_pf_group_dumb(void *priv)
                }
        }
 
-       pfg = create_pfg();
+       pfg = pfg_create();
        if (pfg == NULL)
                goto unlock;
 
        set_pf_dumb(&pfg->filter, priv);
 
-       add_pfg_by_list(pfg);
+       pfg_add_to_list(pfg);
 
 unlock:
        write_unlock(&pfg_list_lock);
@@ -396,10 +393,10 @@ void put_pf_group(struct pf_group *pfg)
 {
        if (atomic_dec_and_test(&pfg->usage)) {
                write_lock(&pfg_list_lock);
-               del_pfg_by_list(pfg);
+               pfg_del_from_list(pfg);
                write_unlock(&pfg_list_lock);
 
-               free_pfg(pfg);
+               pfg_free(pfg);
        }
 }
 EXPORT_SYMBOL_GPL(put_pf_group);
@@ -461,21 +458,6 @@ unlock:
        return ret;
 }
 
-static int pfg_add_proc(struct pf_group *pfg, struct sspt_proc *proc)
-{
-       struct pl_struct *pls;
-
-       pls = create_pl_struct(proc);
-       if (pls == NULL)
-               return -ENOMEM;
-
-       spin_lock(&pfg->pl_lock);
-       add_pl_struct(pfg, pls);
-       spin_unlock(&pfg->pl_lock);
-
-       return 0;
-}
-
 enum pf_inst_flag {
        PIF_NONE,
        PIF_FIRST,
@@ -589,28 +571,14 @@ void call_page_fault(struct task_struct *task, unsigned long page_addr)
 void uninstall_proc(struct sspt_proc *proc)
 {
        struct task_struct *task = proc->task;
-       struct pf_group *pfg;
-       struct pl_struct *pls;
 
-       read_lock(&pfg_list_lock);
-       list_for_each_entry(pfg, &pfg_list, list) {
-               spin_lock(&pfg->pl_lock);
-               pls = find_pl_struct(pfg, task);
-               if (pls) {
-                       del_pl_struct(pls);
-                       free_pl_struct(pls);
-               }
-               spin_unlock(&pfg->pl_lock);
-       }
-       read_unlock(&pfg_list_lock);
 
        task_lock(task);
        BUG_ON(task->mm == NULL);
        sspt_proc_uninstall(proc, task, US_UNREGS_PROBE);
        task_unlock(task);
 
-       sspt_proc_del_all_filters(proc);
-       sspt_proc_free(proc);
+       sspt_proc_cleanup(proc);
 }
 
 /**

diff --git a/us_manager/sspt/sspt_proc.c b/us_manager/sspt/sspt_proc.c
index ce1e024..01f0353 100644 (file)
--- a/us_manager/sspt/sspt_proc.c
+++ b/us_manager/sspt/sspt_proc.c
@@ -105,6 +105,7 @@ struct sspt_proc *sspt_proc_create(struct task_struct *task)
                proc->private_data = NULL;
                INIT_LIST_HEAD(&proc->file_list);
                INIT_LIST_HEAD(&proc->filter_list);
+               atomic_set(&proc->usage, 1);
 
                /* add to list */
                list_add(&proc->list, &proc_probes_list);
@@ -121,12 +122,11 @@ struct sspt_proc *sspt_proc_create(struct task_struct *task)
  */
 
 /* called with sspt_proc_write_lock() */
-void sspt_proc_free(struct sspt_proc *proc)
+void sspt_proc_cleanup(struct sspt_proc *proc)
 {
        struct sspt_file *file, *n;
 
-       /* delete from list */
-       list_del(&proc->list);
+       sspt_proc_del_all_filters(proc);
 
        list_for_each_entry_safe(file, n, &proc->file_list, list) {
                list_del(&file->list);
@@ -136,7 +136,20 @@ void sspt_proc_free(struct sspt_proc *proc)
        sspt_destroy_feature(proc->feature);
 
        free_sm_us(proc->sm);
-       kfree(proc);
+       sspt_proc_put(proc);
+}
+
+struct sspt_proc *sspt_proc_get(struct sspt_proc *proc)
+{
+       atomic_inc(&proc->usage);
+
+       return proc;
+}
+
+void sspt_proc_put(struct sspt_proc *proc)
+{
+       if (atomic_dec_and_test(&proc->usage))
+               kfree(proc);
 }
 
 /**
@@ -213,9 +226,10 @@ struct sspt_proc *sspt_proc_get_by_task_or_new(struct task_struct *task)
 void sspt_proc_free_all(void)
 {
        struct sspt_proc *proc, *n;
+
        list_for_each_entry_safe(proc, n, &proc_probes_list, list) {
-               sspt_proc_del_all_filters(proc);
-               sspt_proc_free(proc);
+               list_del(&proc->list);
+               sspt_proc_cleanup(proc);
        }
 }
 

diff --git a/us_manager/sspt/sspt_proc.h b/us_manager/sspt/sspt_proc.h
index 0f4318c..534f150 100644 (file)
--- a/us_manager/sspt/sspt_proc.h
+++ b/us_manager/sspt/sspt_proc.h
@@ -53,6 +53,7 @@ struct sspt_proc {
        struct list_head filter_list;   /**< Filter list */
        unsigned first_install:1;       /**< Install flag */
        struct sspt_feature *feature;   /**< Ptr to the feature */
+       atomic_t usage;
 
        /* FIXME: for preload (remove those fields) */
        void *private_data;             /**< Process private data */
@@ -64,7 +65,9 @@ struct sspt_proc_cb {
 };
 
 struct sspt_proc *sspt_proc_create(struct task_struct *task);
-void sspt_proc_free(struct sspt_proc *proc);
+void sspt_proc_cleanup(struct sspt_proc *proc);
+struct sspt_proc *sspt_proc_get(struct sspt_proc *proc);
+void sspt_proc_put(struct sspt_proc *proc);
 
 void on_each_proc_no_lock(void (*func)(struct sspt_proc *, void *),
                          void *data);