On Fri, 27 Oct 2017 09:56:40 +0800 Zhou Chengming <[email protected]> wrote:
> Changes from v1: > - We should put the modifies of the kprobe after the re-reg check. > - And then the address_safe check. > - When check_kprobe_address_safe() return fail, the *probed_mod > should be set to NULL, and no module refcount held. Could you split this item from this patch (with initializing probe_mod = NULL), since it is another bug? Thank you, > > Old code use check_kprobe_rereg() to check if the kprobe has been > registered already, but check_kprobe_rereg() will release the > kprobe_mutex then, so maybe two paths will pass the check and > register the same kprobe. This patch put the check inside the mutex. > > Signed-off-by: Zhou Chengming <[email protected]> > --- > kernel/kprobes.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index a1606a4..f622639 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1443,19 +1443,6 @@ static struct kprobe *__get_valid_kprobe(struct kprobe > *p) > return ap; > } > > -/* Return error if the kprobe is being re-registered */ > -static inline int check_kprobe_rereg(struct kprobe *p) > -{ > - int ret = 0; > - > - mutex_lock(&kprobe_mutex); > - if (__get_valid_kprobe(p)) > - ret = -EINVAL; > - mutex_unlock(&kprobe_mutex); > - > - return ret; > -} > - > int __weak arch_check_ftrace_location(struct kprobe *p) > { > unsigned long ftrace_addr; > @@ -1501,6 +1488,7 @@ static int check_kprobe_address_safe(struct kprobe *p, > * its code to prohibit unexpected unloading. > */ > if (unlikely(!try_module_get(*probed_mod))) { > + *probed_mod = NULL; > ret = -ENOENT; > goto out; > } > @@ -1536,9 +1524,13 @@ int register_kprobe(struct kprobe *p) > return PTR_ERR(addr); > p->addr = addr; > > - ret = check_kprobe_rereg(p); > - if (ret) > - return ret; > + mutex_lock(&kprobe_mutex); > + > + /* Return error if the kprobe is being re-registered */ > + if (__get_valid_kprobe(p)) { > + ret = -EINVAL; > + goto out; > + } > > /* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */ > p->flags &= KPROBE_FLAG_DISABLED; > @@ -1547,9 +1539,7 @@ int register_kprobe(struct kprobe *p) > > ret = check_kprobe_address_safe(p, &probed_mod); > if (ret) > - return ret; > - > - mutex_lock(&kprobe_mutex); > + goto out; > > old_p = get_kprobe(p->addr); > if (old_p) { > -- > 1.8.3.1 > -- Masami Hiramatsu <[email protected]>
