On Thu, Apr 05, 2007 at 11:55:17AM +0530, Ananth N Mavinakayanahalli wrote:
> On Wed, Apr 04, 2007 at 11:13:30AM -0700, Keshavamurthy, Anil S wrote:
> > On Wed, Apr 04, 2007 at 05:43:49PM +0530, Ananth N Mavinakayanahalli wrote:
> > > This patch provides a debugfs knob to turn kprobes on/off
> > Nice feature. Thanks. Review inline.
>
> Thanks for the review Anil...
>
> > > +static void __kprobes disable_all_kprobes(void)
> > > +{
> > > + struct hlist_head *head;
> > > + struct hlist_node *node;
> > > + struct kprobe *p;
> > > + unsigned int i;
> > > +
> > > + mutex_lock(&kprobe_mutex);
> > > +
> > > + /* If kprobes are already disabled, just return */
> > > + if (kprobe_enabled == 0)
> > > + goto already_disabled;
> > > +
> > > + kprobe_enabled = 0;
> > > + for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
> > > + head = &kprobe_table[i];
> > > + hlist_for_each_entry_rcu(p, node, head, hlist)
> > > + arch_disarm_kprobe(p);
> > Humm... you are disarming all probes including the probe on
> > kretprobe_trampoline. I think you should not disarm the probe on
> > the kretprobe_trampoline ( which is registerd in arch_init_kprobe()
> > function)
> > as any return probe which has its return address modified to this
> > kretprobe_trampoline address on its stack should be able to recover.
> >
> > Do something like this...
> > hlist_for_each_entry_rcu(p, node, head, hlist)
> > if (!trampoline_probe(p)) /* implement this in arch files */
> > arch_disarm_kprobe(p);
>
> Good catch! Its taken care of now.
>
> > > + }
> > > +
> > > + mutex_unlock(&kprobe_mutex);
> > > + /* Allow all currently running kprobes to complete */
> > > + synchronize_sched();
> > > +
> >
> > [....]
> >
> > > +
> > > +/debug/kprobes/list: Lists all registered probes on the system
> > > +
> > > +c015d71a k vfs_read+0x0 1
> > > +c011a316 j do_fork+0x0 1
> > > +c03dedc5 r tcp_v4_rcv+0x0 1
> > Do you really need to display the last column? As this is global information
> > and not per probe info and user can always read from /debug/kprobes/enabled
> > file.
>
> Agreed.
>
> Updated patch below
>
> This patch provides a debugfs knob to turn kprobes on/off
>
> o A new file /debug/kprobes/enabled indicates if kprobes is enabled or
> not (default enabled)
> o Echoing 0 to this file will disarm all installed probes
> o Any new probe registration when disabled will register the probe but
> not arm it. A message will be printed out in such a case.
> o When a value 1 is echoed to the file, all probes (including ones
> registered in the intervening period) will be enabled
> o Unregistration will happen irrespective of whether probes are globally
> enabled or not.
> o Update Documentation/kprobes.txt to reflect these changes. While there
> also update the doc to make it current.
>
> Patch against 2.6.21-rc5-mm4
>
> We are also looking at providing sysrq key support to tie to the
> disabling feature provided by this patch.
>
> Signed-off-by: Ananth N Mavinakayanahalli <[EMAIL PROTECTED]>
> Signed-off-by: Srinivasa DS <[EMAIL PROTECTED]>
Acked-by: Anil S Keshavamurthy <[EMAIL PROTECTED]>
For some reason my earlier ACK, did not make it to list, re-acking again.
Andrew, please include this patch in your mm tree.
>
> ---
> Documentation/kprobes.txt | 34 ++++++++-
> arch/i386/kernel/kprobes.c | 5 +
> arch/ia64/kernel/kprobes.c | 9 ++
> arch/powerpc/kernel/kprobes.c | 8 ++
> arch/x86_64/kernel/kprobes.c | 8 ++
> include/linux/kprobes.h | 5 +
> kernel/kprobes.c | 154
> ++++++++++++++++++++++++++++++++++++++++--
> 7 files changed, 214 insertions(+), 9 deletions(-)
>
> Index: linux-2.6.21-rc5/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/kernel/kprobes.c
> +++ linux-2.6.21-rc5/kernel/kprobes.c
> @@ -46,6 +46,7 @@
> #include <asm-generic/sections.h>
> #include <asm/cacheflush.h>
> #include <asm/errno.h>
> +#include <asm/uaccess.h>
>
> #define KPROBE_HASH_BITS 6
> #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
> @@ -64,6 +65,9 @@ static struct hlist_head kprobe_table[KP
> static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
> static atomic_t kprobe_count;
>
> +/* NOTE: change this value only with kprobe_mutex held */
> +static bool kprobe_enabled;
> +
> DEFINE_MUTEX(kprobe_mutex); /* Protects kprobe_table */
> DEFINE_SPINLOCK(kretprobe_lock); /* Protects kretprobe_inst_table */
> static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
> @@ -564,11 +568,15 @@ static int __kprobes __register_kprobe(s
> hlist_add_head_rcu(&p->hlist,
> &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);
>
> - if (atomic_add_return(1, &kprobe_count) == \
> + if (kprobe_enabled == 1) {
> + if (atomic_add_return(1, &kprobe_count) == \
> (ARCH_INACTIVE_KPROBE_COUNT + 1))
> - register_page_fault_notifier(&kprobe_page_fault_nb);
> + register_page_fault_notifier(&kprobe_page_fault_nb);
>
> - arch_arm_kprobe(p);
> + arch_arm_kprobe(p);
> + } else
> + printk("Kprobes are globally disabled. This kprobe [@ %p] "
> + "will be enabled with all other probes\n", p->addr);
>
> out:
> mutex_unlock(&kprobe_mutex);
> @@ -607,8 +615,13 @@ valid_p:
> if (old_p == p ||
> (old_p->pre_handler == aggr_pre_handler &&
> p->list.next == &old_p->list && p->list.prev == &old_p->list)) {
> - /* Only probe on the hash list */
> - arch_disarm_kprobe(p);
> + /*
> + * Only probe on the hash list. Disarm only if kprobes are
> + * enabled - otherwise, the breakpoint would already have
> + * been removed. We save on flushing icache.
> + */
> + if (kprobe_enabled == 1)
> + arch_disarm_kprobe(p);
> hlist_del_rcu(&old_p->hlist);
> cleanup_p = 1;
> } else {
> @@ -797,6 +810,9 @@ static int __init init_kprobes(void)
> }
> atomic_set(&kprobe_count, 0);
>
> + /* By default, kprobes are enabled */
> + kprobe_enabled = 1;
> +
> err = arch_init_kprobes();
> if (!err)
> err = register_die_notifier(&kprobe_exceptions_nb);
> @@ -806,7 +822,7 @@ static int __init init_kprobes(void)
>
> #ifdef CONFIG_DEBUG_FS
> static void __kprobes report_probe(struct seq_file *pi, struct kprobe *p,
> - const char *sym, int offset,char *modname)
> + const char *sym, int offset,char *modname)
> {
> char *kprobe_type;
>
> @@ -885,9 +901,128 @@ static struct file_operations debugfs_kp
> .release = seq_release,
> };
>
> +static void __kprobes enable_all_kprobes(void)
> +{
> + struct hlist_head *head;
> + struct hlist_node *node;
> + struct kprobe *p;
> + unsigned int i;
> +
> + mutex_lock(&kprobe_mutex);
> +
> + /* If kprobes are already enabled, just return */
> + if (kprobe_enabled == 1)
> + goto already_enabled;
> +
> + /*
> + * Re-register the page fault notifier only if there are any
> + * active probes at the time of enabling kprobes globally
> + */
> + if (atomic_read(&kprobe_count) > ARCH_INACTIVE_KPROBE_COUNT)
> + register_page_fault_notifier(&kprobe_page_fault_nb);
> +
> + for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
> + head = &kprobe_table[i];
> + hlist_for_each_entry_rcu(p, node, head, hlist)
> + arch_arm_kprobe(p);
> + }
> +
> + kprobe_enabled = 1;
> +
> +already_enabled:
> + mutex_unlock(&kprobe_mutex);
> + return;
> +}
> +
> +static void __kprobes disable_all_kprobes(void)
> +{
> + struct hlist_head *head;
> + struct hlist_node *node;
> + struct kprobe *p;
> + unsigned int i;
> +
> + mutex_lock(&kprobe_mutex);
> +
> + /* If kprobes are already disabled, just return */
> + if (kprobe_enabled == 0)
> + goto already_disabled;
> +
> + kprobe_enabled = 0;
> + for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
> + head = &kprobe_table[i];
> + hlist_for_each_entry_rcu(p, node, head, hlist) {
> + if (!arch_trampoline_kprobe(p))
> + arch_disarm_kprobe(p);
> + }
> + }
> +
> + mutex_unlock(&kprobe_mutex);
> + /* Allow all currently running kprobes to complete */
> + synchronize_sched();
> +
> + mutex_lock(&kprobe_mutex);
> + /* Unconditionally unregister the page_fault notifier */
> + unregister_page_fault_notifier(&kprobe_page_fault_nb);
> +
> +already_disabled:
> + mutex_unlock(&kprobe_mutex);
> + return;
> +}
> +
> +/*
> + * XXX: The debugfs bool file interface doesn't allow for callbacks
> + * when the bool state is switched. We can reuse that facility when
> + * available
> + */
> +static ssize_t read_enabled_file_bool(struct file *file,
> + char __user *user_buf, size_t count, loff_t *ppos)
> +{
> + char buf[3];
> +
> + if (kprobe_enabled)
> + buf[0] = '1';
> + else
> + buf[0] = '0';
> + buf[1] = '\n';
> + buf[2] = 0x00;
> + return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t write_enabled_file_bool(struct file *file,
> + const char __user *user_buf, size_t count, loff_t *ppos)
> +{
> + char buf[32];
> + int buf_size;
> +
> + buf_size = min(count, (sizeof(buf)-1));
> + if (copy_from_user(buf, user_buf, buf_size))
> + return -EFAULT;
> +
> + switch (buf[0]) {
> + case 'y':
> + case 'Y':
> + case '1':
> + enable_all_kprobes();
> + break;
> + case 'n':
> + case 'N':
> + case '0':
> + disable_all_kprobes();
> + break;
> + }
> +
> + return count;
> +}
> +
> +static struct file_operations fops_kp = {
> + .read = read_enabled_file_bool,
> + .write = write_enabled_file_bool,
> +};
> +
> static int __kprobes debugfs_kprobe_init(void)
> {
> struct dentry *dir, *file;
> + unsigned int value = 1;
>
> dir = debugfs_create_dir("kprobes", NULL);
> if (!dir)
> @@ -900,6 +1035,13 @@ static int __kprobes debugfs_kprobe_init
> return -ENOMEM;
> }
>
> + file = debugfs_create_file("enabled", 0600, dir,
> + &value, &fops_kp);
> + if (!file) {
> + debugfs_remove(dir);
> + return -ENOMEM;
> + }
> +
> return 0;
> }
>
> Index: linux-2.6.21-rc5/Documentation/kprobes.txt
> ===================================================================
> --- linux-2.6.21-rc5.orig/Documentation/kprobes.txt
> +++ linux-2.6.21-rc5/Documentation/kprobes.txt
> @@ -14,6 +14,7 @@ CONTENTS
> 8. Kprobes Example
> 9. Jprobes Example
> 10. Kretprobes Example
> +Appendix A: The kprobes debugfs interface
>
> 1. Concepts: Kprobes, Jprobes, Return Probes
>
> @@ -349,9 +350,12 @@ for instrumentation and error reporting.
>
> If the number of times a function is called does not match the number
> of times it returns, registering a return probe on that function may
> -produce undesirable results. We have the do_exit() case covered.
> -do_execve() and do_fork() are not an issue. We're unaware of other
> -specific cases where this could be a problem.
> +produce undesirable results. In such a case, a line:
> +kretprobe BUG!: Processing kretprobe d000000000041aa8 @ c00000000004f48c
> +gets printed. With this information, one will be able to correlate the
> +exact instance of the kretprobe that caused the problem. We have the
> +do_exit() case covered. do_execve() and do_fork() are not an issue.
> +We're unaware of other specific cases where this could be a problem.
>
> If, upon entry to or exit from a function, the CPU is running on
> a stack other than that of the current task, registering a return
> @@ -614,3 +618,27 @@ http://www-106.ibm.com/developerworks/li
> http://www.redhat.com/magazine/005mar05/features/kprobes/
> http://www-users.cs.umn.edu/~boutcher/kprobes/
> http://www.linuxsymposium.org/2006/linuxsymposium_procv2.pdf (pages 101-115)
> +
> +
> +Appendix A: The kprobes debugfs interface
> +
> +With recent kernels (> 2.6.20) the list of registered kprobes is visible
> +under the /debug/kprobes/ directory (assuming debugfs is mounted at /debug).
> +
> +/debug/kprobes/list: Lists all registered probes on the system
> +
> +c015d71a k vfs_read+0x0
> +c011a316 j do_fork+0x0
> +c03dedc5 r tcp_v4_rcv+0x0
> +
> +The first column provides the kernel address where the probe is inserted.
> +The second column identifies the type of probe (k - kprobe, r - kretprobe
> +and j - jprobe), while the third column specifies the symbol+offset of
> +the probe. If the probed function belongs to a module, the module name
> +is also specified.
> +
> +/debug/kprobes/enabled: Turn kprobes ON/OFF
> +
> +Provides a knob to globally turn registered kprobes ON or OFF. By default,
> +all kprobes are enabled. By echoing "0" to this file, all registered probes
> +will be disarmed, till such time a "1" is echoed to this file.
> Index: linux-2.6.21-rc5/arch/i386/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/arch/i386/kernel/kprobes.c
> +++ linux-2.6.21-rc5/arch/i386/kernel/kprobes.c
> @@ -748,6 +748,11 @@ int __kprobes longjmp_break_handler(stru
> return 0;
> }
>
> +int __kprobes arch_trampoline_kprobe(struct kprobe *p)
> +{
> + return 0;
> +}
> +
> int __init arch_init_kprobes(void)
> {
> return 0;
> Index: linux-2.6.21-rc5/arch/ia64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/arch/ia64/kernel/kprobes.c
> +++ linux-2.6.21-rc5/arch/ia64/kernel/kprobes.c
> @@ -1016,3 +1016,12 @@ int __init arch_init_kprobes(void)
> (kprobe_opcode_t *)((struct fnptr *)kretprobe_trampoline)->ip;
> return register_kprobe(&trampoline_p);
> }
> +
> +int __kprobes arch_trampoline_kprobe(struct kprobe *p)
> +{
> + if (p->addr ==
> + (kprobe_opcode_t *)((struct fnptr *)kretprobe_trampoline)->ip)
> + return 1;
> +
> + return 0;
> +}
> Index: linux-2.6.21-rc5/arch/powerpc/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/arch/powerpc/kernel/kprobes.c
> +++ linux-2.6.21-rc5/arch/powerpc/kernel/kprobes.c
> @@ -521,3 +521,11 @@ int __init arch_init_kprobes(void)
> {
> return register_kprobe(&trampoline_p);
> }
> +
> +int __kprobes arch_trampoline_kprobe(struct kprobe *p)
> +{
> + if (p->addr == (kprobe_opcode_t *)&kretprobe_trampoline)
> + return 1;
> +
> + return 0;
> +}
> Index: linux-2.6.21-rc5/arch/x86_64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/arch/x86_64/kernel/kprobes.c
> +++ linux-2.6.21-rc5/arch/x86_64/kernel/kprobes.c
> @@ -748,3 +748,11 @@ int __init arch_init_kprobes(void)
> {
> return register_kprobe(&trampoline_p);
> }
> +
> +int __kprobes arch_trampoline_kprobe(struct kprobe *p)
> +{
> + if (p->addr == (kprobe_opcode_t *)&kretprobe_trampoline)
> + return 1;
> +
> + return 0;
> +}
> Index: linux-2.6.21-rc5/include/linux/kprobes.h
> ===================================================================
> --- linux-2.6.21-rc5.orig/include/linux/kprobes.h
> +++ linux-2.6.21-rc5/include/linux/kprobes.h
> @@ -125,11 +125,16 @@ DECLARE_PER_CPU(struct kprobe_ctlblk, kp
> #ifdef ARCH_SUPPORTS_KRETPROBES
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
> +extern int arch_trampoline_kprobe(struct kprobe *p);
> #else /* ARCH_SUPPORTS_KRETPROBES */
> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
> struct pt_regs *regs)
> {
> }
> +static inline int arch_trampoline_kprobe(struct kprobe *p)
> +{
> + return 0;
> +}
> #endif /* ARCH_SUPPORTS_KRETPROBES */
> /*
> * Function-return probe -
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
