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
| ||
|
Date: Wed, 05 Nov 2008 14:46:07 -0800 From: Hiroshi Shimamoto <h-shimamoto@...jp.nec.com> To: Masami Hiramatsu <mhiramat@...hat.com> Cc: Lai Jiangshan <laijs@...fujitsu.com>, Andrew Morton <akpm@...ux-foundation.org>, ananth@...ibm.com, David Miller <davem@...emloft.net>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] kprobes: disable preempt for module_text_address() Masami Hiramatsu wrote: > Lai Jiangshan wrote: >> actually, calling __module_text_address() in __register_kprobe() is >> better after my fix applied. but I found that a line have exceed >> 80 characters, so I don't use __module_text_address(). > > I don't think that coding style is a good reason not to fix it...:( > > Anyway, I think the issue that you pointed must be fixed. > I found there were same kind of issues in kprobes and updated > your patch. This includes fixes which Hiroshi pointed out. > > Thanks a lot! :) > > __register_kprobe() can be preempted after checking probing address > but before try_module_get() or module_put(), and in this interval the > module can be unloaded. In that case, try_module_get(probed_mod) or > module_put(mod) will access to invalid address, or kprobe will probe > invalid address. > > this patch uses preempt_disable() to protect it and use > __module_text_address() and __kernel_text_address(). > > Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com> > Signed-off-by: Masami Hiramatsu <mhiramat@...hat.com> > --- > kernel/kprobes.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > Index: 2.6.28-rc3/kernel/kprobes.c > =================================================================== > --- 2.6.28-rc3.orig/kernel/kprobes.c > +++ 2.6.28-rc3/kernel/kprobes.c > @@ -613,30 +613,37 @@ static int __kprobes __register_kprobe(s > return -EINVAL; > p->addr = addr; > > - if (!kernel_text_address((unsigned long) p->addr) || > - in_kprobes_functions((unsigned long) p->addr)) > + preempt_disable(); > + if (!__kernel_text_address((unsigned long) p->addr) || > + in_kprobes_functions((unsigned long) p->addr)) { > + preempt_enable(); > return -EINVAL; > + } > > p->mod_refcounted = 0; > > /* > * Check if are we probing a module. > */ > - probed_mod = module_text_address((unsigned long) p->addr); > + probed_mod = __module_text_address((unsigned long) p->addr); > if (probed_mod) { > - struct module *calling_mod = module_text_address(called_from); > + struct module *calling_mod; > + calling_mod = __module_text_address(called_from); > /* > * We must allow modules to probe themself and in this case > * avoid incrementing the module refcount, so as to allow > * unloading of self probing modules. > */ > if (calling_mod && calling_mod != probed_mod) { One question, off topic. If calling_mod is NULL, no try_module_get(), is that OK? thanks, Hiroshi Shimamoto > - if (unlikely(!try_module_get(probed_mod))) > + if (unlikely(!try_module_get(probed_mod))) { > + preempt_enable(); > return -EINVAL; > + } > p->mod_refcounted = 1; > } else > probed_mod = NULL; > } > + preempt_enable(); > > p->nmissed = 0; > INIT_LIST_HEAD(&p->list); > @@ -718,9 +725,11 @@ static void __kprobes __unregister_kprob > struct kprobe *old_p; > > if (p->mod_refcounted) { > - mod = module_text_address((unsigned long)p->addr); > + preempt_disable(); > + mod = __module_text_address((unsigned long)p->addr); > if (mod) > module_put(mod); > + preempt_enable(); > } > > if (list_empty(&p->list) || list_is_singular(&p->list)) { > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists