On Thu, 26 Oct 2017 20:11:25 +0800 Zhou Chengming <[email protected]> wrote:
> 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. Still no good, see below comment. > > Signed-off-by: Zhou Chengming <[email protected]> > --- > kernel/kprobes.c | 23 ++++++----------------- > 1 file changed, 6 insertions(+), 17 deletions(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index a1606a4..2a4873a 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; > @@ -1536,10 +1523,6 @@ int register_kprobe(struct kprobe *p) > return PTR_ERR(addr); > p->addr = addr; > > - ret = check_kprobe_rereg(p); > - if (ret) > - return ret; > - > /* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */ > p->flags &= KPROBE_FLAG_DISABLED; > p->nmissed = 0; here, we already modifies the kprobe. We need to check and reject before modifying it. Thank you, > @@ -1551,6 +1534,12 @@ int register_kprobe(struct kprobe *p) > > mutex_lock(&kprobe_mutex); > > + /* Return error if the kprobe is being re-registered */ > + if (__get_valid_kprobe(p)) { > + ret = -EINVAL; > + goto out; > + } > + > old_p = get_kprobe(p->addr); > if (old_p) { > /* Since this may unoptimize old_p, locking text_mutex. */ > -- > 1.8.3.1 > -- Masami Hiramatsu <[email protected]>
