Messages in this thread | | | Date | Mon, 4 Dec 2017 17:35:15 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH v2] kernel/kprobes: add re-register safe check for register_kretprobe() |
| |
On Fri, 1 Dec 2017 20:09:06 +0800 JianKang Chen <chenjiankang1@huawei.com> wrote:
> From: Jiankang Chen <chenjiankang1@huawei.com> > > When there are two same struct kretprobe rp, the INIT_HLIST_HEAD() > will result in a empty list table rp->free_instances. The memory leak > will happen. So it needs to add re-register safe check by > __get_valid_kprobe(). > > However, current this is not safe for multi-threadings, because > there is still a chance to re-register kretprobe concurrently. > So I add a kretprobe_mutex lock to protect the INIT_LIST_HEAD > > And we use rcu read lock to protect the rcu list for __get_valid_kprobe
Looks good to me :)
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
> > Signed-off-by: Jiankang Chen <chenjiankang1@huawei.com> > --- > kernel/kprobes.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index a1606a4..f8f027a 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -67,6 +67,7 @@ > > /* This protects kprobe_table and optimizing_list */ > static DEFINE_MUTEX(kprobe_mutex); > +static DEFINE_MUTEX(kretprobe_mutex); > static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL; > static struct { > raw_spinlock_t lock ____cacheline_aligned_in_smp; > @@ -1919,6 +1920,7 @@ int register_kretprobe(struct kretprobe *rp) > struct kretprobe_instance *inst; > int i; > void *addr; > + struct kprobe *kp; > > if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset)) > return -EINVAL; > @@ -1947,6 +1949,15 @@ int register_kretprobe(struct kretprobe *rp) > rp->maxactive = num_possible_cpus(); > #endif > } > + > + mutex_lock(&kretprobe_mutex); > + rcu_read_lock(); > + kp = __get_valid_kprobe(&rp->kp); > + rcu_read_unlock(); > + if (kp) { > + ret = -EINVAL; > + goto out; > + } > raw_spin_lock_init(&rp->lock); > INIT_HLIST_HEAD(&rp->free_instances); > for (i = 0; i < rp->maxactive; i++) { > @@ -1954,7 +1965,8 @@ int register_kretprobe(struct kretprobe *rp) > rp->data_size, GFP_KERNEL); > if (inst == NULL) { > free_rp_inst(rp); > - return -ENOMEM; > + ret = -ENOMEM; > + goto out; > } > INIT_HLIST_NODE(&inst->hlist); > hlist_add_head(&inst->hlist, &rp->free_instances); > @@ -1965,6 +1977,8 @@ int register_kretprobe(struct kretprobe *rp) > ret = register_kprobe(&rp->kp); > if (ret != 0) > free_rp_inst(rp); > +out: > + mutex_unlock(&kretprobe_mutex); > return ret; > } > EXPORT_SYMBOL_GPL(register_kretprobe); > -- > 1.7.12.4 >
-- Masami Hiramatsu <mhiramat@kernel.org>
| |