lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-Id: <20171027145721.ef28a8eea41210f05c23f30e@kernel.org> Date: Fri, 27 Oct 2017 14:57:21 +0900 From: Masami Hiramatsu <mhiramat@...nel.org> To: Zhou Chengming <zhouchengming1@...wei.com> Cc: <ananth@...ux.vnet.ibm.com>, <anil.s.keshavamurthy@...el.com>, <davem@...emloft.net>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v2] kprobes: avoid the kprobe being re-registered On Fri, 27 Oct 2017 09:56:40 +0800 Zhou Chengming <zhouchengming1@...wei.com> 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 <zhouchengming1@...wei.com> > --- > 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 <mhiramat@...nel.org>
Powered by blists - more mailing lists