Bibo wrote: >> I wrote one module named probed.ko after compiled, and the other >> is kprobe module named probing.ko which is to probe some function >> defined in probed.ko module. >> [...] >> #insmod probed.ko >> #insmod probing.ko >> #rmmod probed.ko >> #rmmod probing.ko Frank wrote: >> [...] >To manage module reference counts in a way that prevents this problem, >systemtap keeps a file descriptor open on some file under >/sys/module/<PROBED>/. By using plain insmod, you are giving up this >protection. Anil wrote: I think we should implement this module reference count logic built in the kernel kprobes code. Here is how it can be done. register_kprobe(...) { ..... + if ((mod = module_text_address((unsigned long) p->addr))) + if (unlikely(!try_module_get(mod))) return -EINVAL; .... } unregister_kprobe(...) { .... + module_put(module_text_address((unsigned long)p->addr)); ... } Will post the actual patch later.
Created attachment 771 [details] now takes the reference count Patch received from Mao Bibo.
Yes, perhaps kprobes could do this for us. The translator-generated code cannot, for that handy module_text_address symbol is not exported to modules. The future cross-instrumentation work (bug #1145) is likely to render this moot, since it will require even more module information digging at initialization time.
Anil, do you want to own this one?
(In reply to comment #3) > Anil, do you want to own this one? Sure, I can own this and you should see the final patch on lkml tomorrow. -Anil
Mao Bibo has posted a patch to LKML, acked by Anil: http://www.uwsg.indiana.edu/hypermail/linux/kernel/0511.3/1501.html
I was unable to rmmod the module, where this module had insertd probes one iof its own functions. This is because we reference count this modules and rmmod will never invoke module_exit() routine.
(In reply to comment #6) > I was unable to rmmod the module, where this module had insertd probes one iof > its own functions. This is because we reference count this modules and rmmod > will never invoke module_exit() routine. If when module function will be probe by register_kprobe() function, we do not plus this module's refcount by 1. And then when unregister_kprobe() is called, it will recover to original intruction, if probed address remain in kernel address space, instruction will be recovered, and if it is not in kernel address space, instrunction recovery will be skipped. Is that ok?
(In reply to comment #8) > (In reply to comment #6) > Is that ok? Yes, should be okay. please attach the patch.
> Yes, should be okay. please attach the patch. There is another problem, after removing one module another module can insmod, the text address space maybe is the same. And there will be problem if unregister the kprobe. And I will think over this problem carefully.
I think that probing function in the same module should be prohibited. Here is the patch.It is based on linux-2.6.15-rc5-mm3 kernel version. diff -Nruap linux-2.6.15-rc5-mm3/kernel/kprobes.c linux-2.6.15-rc5- mm3.new/kernel/kprobes.c --- linux-2.6.15-rc5-mm3/kernel/kprobes.c 2005-12-23 09:57:17.000000000 +0800 +++ linux-2.6.15-rc5-mm3.new/kernel/kprobes.c 2005-12-23 09:53:10.000000000 +0800 @@ -454,14 +454,19 @@ int __kprobes register_kprobe(struct kpr int ret = 0; struct kprobe *old_p; struct module *mod; + unsigned long called_from; if ((!kernel_text_address((unsigned long) p->addr)) || in_kprobes_functions((unsigned long) p->addr)) return -EINVAL; - if ((mod = module_text_address((unsigned long) p->addr)) && - (unlikely(!try_module_get(mod)))) - return -EINVAL; + if (mod = module_text_address((unsigned long) p->addr)){ + called_from = __builtin_return_address(0); + if (mod == module_text_address(called_from)) + return -EINVAL; + if (unlikely(!try_module_get(mod))) + return -EINVAL; + } p->nmissed = 0; down(&kprobe_mutex);
Bibo, Your patch does resolve the problem caused by the direct call to register_kprobe. But how about the indirect call? For example, If module A calls a function in module B, the function calls register_kprobe to module A, it'll be unable to remove module A. To resolve it thoroughly, how about to add a new member, suck like flag, to struct kprobe? When bit 0 of kprobe->flag is set, register_kprobe doesn't increase the module ref count. Anil, Any idea? Yanmin
Regarding Comment #11, I don't think we should disallow a module probing itself. I know of some kprobes performance tests that do exactly that: the module contains a dummy function, registers a probe on that function, then calls that function repeatedly in a tight loop. I think that the correct approach is to detect that the module is probing itself, and just refrain from incrementing the reference count in that case.
Looks like Bibo's 11/30/05 patch is in 2.6.15.
Created attachment 814 [details] Avoid refcouning for self probed modules Ananth and Jim, I have incroprated both of your suggestings and fixes. Though the patch works, I think this needs more through code review and testing.
Fixed in 2.6.16-rc1.