Redo report_signal callback(). 1. Ensure we dont mistake signal_action just because another engine's report_signal callback has messed with UTRACE_SIGNAL_REPORT. i.e check orig_ka.
2. gdbstub works similarly irrespective of the order of the engines attached. If gdbstub attaches to a process thats already traced with uprobes then uprobes engine report_* callbacks tend to be called before gdbstub report_* callbacks. 3. This patch adds a additional field in gdb_connection current_gdb_uprobe that tracks the gdb_uprobe struct for the active uprobe. 4. This patch also adds find_gdb_uprobe() and find_gdb_connection helper functions. 5. No need to lock gdb_connections_mutex when unregistering uprobes. Signed-off-by: Srikar Dronamraju <sri...@linux.vnet.ibm.com> --- kernel/utrace-gdb.c | 334 +++++++++++++++++++++++++++++++-------------------- 1 files changed, 202 insertions(+), 132 deletions(-) diff --git a/kernel/utrace-gdb.c b/kernel/utrace-gdb.c index ac653fe..f8a07af 100644 --- a/kernel/utrace-gdb.c +++ b/kernel/utrace-gdb.c @@ -34,8 +34,10 @@ #include <linux/regset.h> #include <linux/utrace.h> #include <linux/tracehook.h> +#ifdef CONFIG_HAVE_UPROBES #include <linux/uprobes.h> #include <linux/ubp.h> +#endif /** struct gdb_connection - Tracks one active gdb-process session. @@ -63,7 +65,18 @@ struct gdb_connection { int pass_signals; int stop_signals; /* XXX: per-thread later */ + +#ifdef CONFIG_HAVE_UPROBES + /* + * gdb_engine need not always be attached to the process before + * uprobes_engine. If uprobe's engine report_signal callback is + * called first, it would change the instruction address. Checking + * if the corresponding uprobe is disarmed can be easily be done by + * caching the gdb_uprobe pointer. + */ + struct gdb_uprobe *current_gdb_uprobe; /* XXX: per-thread later */ struct list_head uprobes; +#endif char output_buf[GDB_BUFMAX]; size_t output_buf_size; @@ -228,36 +241,78 @@ static void push_output_packet (struct gdb_connection *p, const char *s) push_output_packet_end(p, start); } +static inline struct gdb_connection *find_gdb_connection( + struct task_struct *task) +{ + struct gdb_connection *gc = NULL; + mutex_lock(&gdb_connections_mutex); + list_for_each_entry(gc, &gdb_connections, link) { + if (gc->target == task_tgid_nr(task)) + goto out_connection; + } + +out_connection: + mutex_unlock(&gdb_connections_mutex); + return gc; +} /* ------------------------------------------------------------------------ */ -static bool can_gdb_handle_sigtrap(struct pt_regs *regs, +#ifdef CONFIG_HAVE_UPROBES + +static inline struct gdb_uprobe *find_gdb_uprobe(unsigned long addr, struct gdb_connection *p) { - unsigned long bkpt = ubp_get_bkpt_addr (regs); - struct task_struct *task = current; + struct gdb_uprobe *gup, *gup2; - if (p->target == task_tgid_nr(task)) { - struct list_head *l; - struct list_head *l2; - struct gdb_uprobe *gup = NULL; + list_for_each_entry_safe(gup, gup2, &p->uprobes, link) { + pr_debug("considering addr=%p %s\n", (void *)gup->up.vaddr, + gup->disarmed_p ? "disarmed" : ""); + if (gup->up.vaddr == addr) + return gup; + } + return NULL; +} - list_for_each_safe(l, l2, &p->uprobes) { - gup = list_entry(l, struct gdb_uprobe, link); - pr_debug("considering addr=%p %s\n", - (void *)gup->up.vaddr, - gup->disarmed_p ? "disarmed" : ""); - if (gup->up.vaddr == bkpt && !gup->disarmed_p) - return true; - } +/* + * If no breakpoint, return false; + * If breakpoint and breakpoint is disarmed; return true + * If breakpoint and breakpoint is not disarmed; return false + */ +static inline bool is_bkpt_disarmed(struct pt_regs *regs, + struct gdb_connection *p) +{ + struct gdb_uprobe *gup = p->current_gdb_uprobe; + + if (!gup) { + unsigned long addr = ubp_get_bkpt_addr(regs); + + /* + * gup will be non-NULL iff uprobe engine report_signal + * callback was called before gdbstub engine's report_signal + * callback. i.e gdb_uprobe_handler was just called before + * calling this report_signal callback. + */ + gup = find_gdb_uprobe(addr, p); } + if (gup) + return gup->disarmed_p; + return false; } -#ifdef CONFIG_HAVE_UPROBES /* uprobe callback */ void gdb_uprobe_handler(struct uprobe *up, struct pt_regs *regs) { + struct gdb_connection *gc; + struct gdb_uprobe *gup; + + gc = find_gdb_connection(current); + if (!gc) + return; + + gup = container_of(up, struct gdb_uprobe, up); + gc->current_gdb_uprobe = gup; } void gdb_register_complete(struct uprobe *up, int regflag, int ret) @@ -292,12 +347,8 @@ void gdb_register_complete(struct uprobe *up, int regflag, int ret) } #endif /* CONFIG_HAVE_UPROBES */ - - - /* utrace callbacks */ - u32 gdb_utrace_report_quiesce(enum utrace_resume_action action, struct utrace_engine *engine, struct task_struct *task, @@ -351,13 +402,15 @@ u32 gdb_utrace_report_exec(enum utrace_resume_action action, struct list_head *l; struct list_head *l2; list_for_each_safe(l, l2, & p->uprobes) { - struct gdb_uprobe *gup = list_entry (l, struct gdb_uprobe, link); - /* - int rc = unmap_uprobe (& gup->up); - WARN_ON (rc != 0); - */ - list_del (& gup->link); - kfree (gup); + struct gdb_uprobe *gup = list_entry(l, + struct gdb_uprobe, link); + unregister_uprobe(&gup->up); + /* + * int rc = unmap_uprobe(& gup->up); + * WARN_ON(rc != 0); + */ + list_del(&gup->link); + kfree(gup); } } #endif /* CONFIG_HAVE_UPROBES */ @@ -390,26 +443,28 @@ u32 gdb_utrace_report_signal(u32 action, pr_debug ("report_signal %d (0x%x) kern %d skip %d stop %d\n", task->pid, action, kern_p, p->pass_signals, p->stop_signals); - /* The target is about to receive a signal. There are several - * cases: - * - * 1) This is an ordinary signal. We UTRACE_STOP to notify gdb. - * - * 2a) This is a SIGTRAP arising from a uprobe. We UTRACE_RESUME, since - * we'll receive a uprobe_handler callback shortly, and can - * UTRACE_REPORT (with quiesce->stop) there. - * - * 2b) This is a SIGTRAP arising from a breakpoint, but not from a - * uprobe. We UTRACE_STOP to pass it to gdb. - * - * 3) This is a UTRACE_SIGNAL_REPORT our code injected to stop the process, - * as per UTRACE_INTERRUPT. We UTRACE_STOP | UTRACE_SIGNAL_IGN. - * - * 4) This is a signal our code injected on behalf of gdb via the C/S/I packets. - * We recognize this from p->pass_signals. We UTRACE_RESUME. - * - * 5) This is a UTRACE_SIGNAL_HANDLER event. UTRACE_RESUME. - */ + /* The target is about to receive a signal. There are several + * cases: + * + * 1) This is an ordinary signal. We UTRACE_STOP to notify gdb. + * + * 2a) This is a SIGTRAP arising from a singlestep arising from + * another tracing entity. Or a breakpoint is disarmed. + * We UTRACE_RESUME. + * + * 2b) This is a SIGTRAP but not from a singlestep. + * We UTRACE_STOP to pass it to gdb. + * + * 3) This is a UTRACE_SIGNAL_REPORT our code injected to stop the + * process, as per UTRACE_INTERRUPT. + * We UTRACE_STOP | UTRACE_SIGNAL_IGN. + * + * 4) This is a signal our code injected on behalf of gdb via the + * C/S/I packets. We recognize this from p->pass_signals. + * We UTRACE_RESUME. + * + * 5) This is a UTRACE_SIGNAL_HANDLER event. UTRACE_RESUME. + */ switch (utrace_signal_action(action)) { case UTRACE_SIGNAL_HANDLER: /* case 5 */ @@ -417,44 +472,69 @@ u32 gdb_utrace_report_signal(u32 action, ret = UTRACE_RESUME | utrace_signal_action(action); break; - case UTRACE_SIGNAL_REPORT: /* case 3 */ - if (p->stop_signals > 0) { - p->stop_signals = 0; - snprintf (p->stopcode, GDB_BUFMAX, "S%02x", 2); /* SIGINT */ - push_output_packet (p, p->stopcode); - p->at_quiesce_do = UTRACE_STOP; - ret = UTRACE_STOP | UTRACE_SIGNAL_IGN; - } else { - ret = p->at_quiesce_do | utrace_signal_action(action); - } - break; + case UTRACE_SIGNAL_REPORT: + case UTRACE_SIGNAL_DELIVER: + case UTRACE_SIGNAL_IGN: /* XXX: bother notify? */ + case UTRACE_SIGNAL_TERM: + case UTRACE_SIGNAL_CORE: + case UTRACE_SIGNAL_STOP: + case UTRACE_SIGNAL_TSTP: + if (!orig_ka) { /* case 3a */ + /* This should be UTRACE_SIGNAL_REPORT */ + if (p->stop_signals > 0) { + p->stop_signals = 0; + /* Indicate as if thread received a SIGINT*/ + snprintf(p->stopcode, GDB_BUFMAX, "S%02x", 2); + push_output_packet(p, p->stopcode); + p->at_quiesce_do = UTRACE_STOP; + ret = UTRACE_STOP | UTRACE_SIGNAL_IGN; + } else + ret = p->at_quiesce_do | + utrace_signal_action(action); - case UTRACE_SIGNAL_DELIVER: - case UTRACE_SIGNAL_IGN: /* XXX: bother notify? */ - case UTRACE_SIGNAL_TERM: - case UTRACE_SIGNAL_CORE: - case UTRACE_SIGNAL_STOP: - case UTRACE_SIGNAL_TSTP: + break; + } + if (info->si_signo == SIGTRAP) { + if (test_thread_flag(TIF_SINGLESTEP) && + !p->stop_signals) + /* + * Case 2a: Thread is singlestepping, + * because some tracing utility other than + * gdbstub requested singlestep. Lets hand + * it over to them. + */ + ret = UTRACE_RESUME | + utrace_signal_action(action); #ifdef CONFIG_HAVE_UPROBES - if (info->si_signo == SIGTRAP) { /* case 2a */ - if (can_gdb_handle_sigtrap(regs, p)) { + else if (is_bkpt_disarmed(regs, p)) + /* + * Case 2a: Thread has hit a breakpoint but + * breakpoint is disarmed. + */ + ret = UTRACE_RESUME | + utrace_signal_action(action); +#endif /* CONFIG_HAVE_UPROBES */ + else { + /* + * Case 2b: Thread has hit a breakpoint or + * delivered a SIGTRAP signal. We need to + * know why? + */ p->at_quiesce_do = UTRACE_STOP; /* SIGTRAP */ snprintf(p->stopcode, GDB_BUFMAX, "S%02x", 5); push_output_packet(p, p->stopcode); ret = UTRACE_STOP | utrace_signal_action(action); - } else - ret = UTRACE_RESUME | - utrace_signal_action(action); + } + p->current_gdb_uprobe = NULL; break; } -#endif /* CONFIG_HAVE_UPROBES */ if (p->pass_signals > 0 /*&& kern_p*/) { /* case 4 */ p->pass_signals --; p->at_quiesce_do = UTRACE_RESUME; ret = UTRACE_RESUME | utrace_signal_action (action); /* deliver */ - } else { /* case 1, case 2b */ + } else { /* case 1 */ snprintf (p->stopcode, GDB_BUFMAX, "S%02x", map_signal_kern2gdb(info->si_signo)); push_output_packet (p, p->stopcode); @@ -494,13 +574,15 @@ u32 gdb_utrace_report_exit(enum utrace_resume_action action, struct list_head *l; struct list_head *l2; list_for_each_safe(l, l2, & p->uprobes) { - struct gdb_uprobe *gup = list_entry (l, struct gdb_uprobe, link); - /* - int rc = unmap_uprobe (& gup->up); - WARN_ON (rc != 0); - */ - list_del (& gup->link); - kfree (gup); + struct gdb_uprobe *gup = list_entry(l, + struct gdb_uprobe, link); + unregister_uprobe(&gup->up); + /* + * int rc = unmap_uprobe(& gup->up); + * WARN_ON(rc != 0); + */ + list_del(&gup->link); + kfree(gup); } } #endif /* CONFIG_HAVE_UPROBES */ @@ -533,13 +615,15 @@ u32 gdb_utrace_report_death(struct utrace_engine *engine, struct list_head *l; struct list_head *l2; list_for_each_safe(l, l2, & p->uprobes) { - struct gdb_uprobe *gup = list_entry (l, struct gdb_uprobe, link); - /* - int rc = unmap_uprobe (& gup->up); - WARN_ON (rc != 0); - */ - list_del (& gup->link); - kfree (gup); + struct gdb_uprobe *gup = list_entry(l, + struct gdb_uprobe, link); + unregister_uprobe(&gup->up); + /* + * int rc = unmap_uprobe(& gup->up); + * WARN_ON(rc != 0); + */ + list_del(&gup->link); + kfree(gup); } } #endif /* CONFIG_HAVE_UPROBES */ @@ -1071,26 +1155,18 @@ static void handle_gdb_command_packet (struct gdb_connection *p, struct task_str case 'Z': /* TYPE,ADDR,LENGTH */ rc = sscanf(& p->input_buf[2], "%lu,%lx,%lx", &arg1, &arg2, &arg3); pr_debug ("'Z' %lu %lx %lx\n", arg1, arg2, arg3); - if (rc != 3 || (arg1 > 1) /*|| (arg3 != 1)*/) { - push_output_packet(p, "E03"); - } else { - struct list_head *l; - struct list_head *l2; + if (rc != 3 || (arg1 > 1) /*|| (arg3 != 1)*/) + push_output_packet(p, "E03"); + else { struct gdb_uprobe *gup = NULL; - list_for_each_safe(l, l2, & p->uprobes) { - gup = list_entry (l, struct gdb_uprobe, link); - if (gup->up.vaddr == arg2) { - /* XXX: WARN_ON (gup->disarmed_p == 0) ? */ - gup->disarmed_p = 0; - push_output_packet (p, "OK"); - break; - } else { - gup = NULL; - } - } - - if (gup) /* breakpoint already exists */ + + gup = find_gdb_uprobe(arg2, p); + if (gup) { + /* XXX: WARN_ON (gup->disarmed_p == 1) ? */ + gup->disarmed_p = 0; + push_output_packet(p, "OK"); break; + } gup = kzalloc(sizeof(struct gdb_uprobe), GFP_KERNEL); if (!gup) { @@ -1117,29 +1193,24 @@ static void handle_gdb_command_packet (struct gdb_connection *p, struct task_str case 'z': /* TYPE,ADDR,LENGTH */ rc = sscanf(& p->input_buf[2], "%lu,%lx,%lx", &arg1, &arg2, &arg3); pr_debug ("'z' %lu %lx %lx\n", arg1, arg2, arg3); - if (rc != 3 || (arg1 > 1) /* || (arg3 != 1) */) { - push_output_packet(p, "E03"); - } else { - struct list_head *l; - struct list_head *l2; + if (rc != 3 || (arg1 > 1) /* || (arg3 != 1) */) + push_output_packet(p, "E03"); + else { struct gdb_uprobe *gup = NULL; - list_for_each_safe(l, l2, & p->uprobes) { - gup = list_entry (l, struct gdb_uprobe, link); - if (gup->up.vaddr == arg2) { - /* NB: We must not unregister_uprobe() here, - since most likely the thread/process is in UTRACE_STOP - state, with the single-step not even started. */ - /* XXX: WARN_ON (gup->disarmed_p == 1) ? */ - gup->disarmed_p = 1; - break; - } else { - gup = NULL; - } - } - if (gup == NULL) + + gup = find_gdb_uprobe(arg2, p); + if (gup) { + /* + * NB: We must not unregister_uprobe() here, + * since most likely the thread/process is + * in UTRACE_STOP state, with the + * single-step not even started. + */ + /* XXX: WARN_ON (gup->disarmed_p == 1) ? */ + gup->disarmed_p = 1; + push_output_packet(p, "OK"); + } else push_output_packet(p, "E04"); - else /* NB: not dereferencing gup. */ - push_output_packet(p, "OK"); } break; #endif /* CONFIG_HAVE_UPROBES */ @@ -1148,9 +1219,6 @@ static void handle_gdb_command_packet (struct gdb_connection *p, struct task_str } } - - - /* ------------------------------------------------------------------------ */ /* gdb control callbacks */ @@ -1207,12 +1275,14 @@ static int proc_gdb_open(struct inode *inode, struct file *filp) mutex_init(& p->input_mutex); init_waitqueue_head(& p->input_wait); - INIT_LIST_HEAD(&p->uprobes); - p->target = task->tgid; +#ifdef CONFIG_HAVE_UPROBES + INIT_LIST_HEAD(&p->uprobes); +#endif /* NB: During attach, we don't want to bother the target. Soon though a send_sig will interrupt it. */ p->at_quiesce_do = UTRACE_RESUME; + p->target = task->tgid; p->engine = utrace_attach_task(task, UTRACE_ATTACH_CREATE | @@ -1281,6 +1351,9 @@ static int proc_gdb_release(struct inode *inode, struct file *filp) ret = utrace_barrier(task, p->engine); } + list_del(&p->link); + mutex_unlock(&gdb_connections_mutex); + #ifdef CONFIG_HAVE_UPROBES { struct list_head *l, *l2; @@ -1295,11 +1368,8 @@ static int proc_gdb_release(struct inode *inode, struct file *filp) } #endif /* CONFIG_HAVE_UPROBES */ - list_del(&p->link); kfree(p); - mutex_unlock (& gdb_connections_mutex); - return ret; }