On Tue, Jul 07, 2015 at 03:23:02AM +0200, Oleg Nesterov wrote: > Test-case: > > #include <stdio.h> > #include <setjmp.h> > > jmp_buf jmp; > > void func_2(void) > { > longjmp(jmp, 1); > } > > void func_1(void) > { > if (setjmp(jmp)) > return; > func_2(); > printf("ERR!! I am running on the caller's stack\n"); > } > > int main(void) > { > func_1(); > return 0; > } > > fails if you probe func_1() and func_2() because handle_trampoline() > assumes that the probed function should must return and hit the bp > installed be prepare_uretprobe(). But in this case func_2() does not > return, so when func_1() returns the kernel uses the no longer valid > return_instance of func_2(). > > Change handle_trampoline() to unwind ->return_instances until we know > that the next chain is alive or NULL, this ensures that the current > chain is the last we need to report and free. > > Alternatively, every return_instance could use unique trampoline_vaddr, > in this case we could use it as a key. And this could solve the problem > with sigaltstack() automatically. > > But this approach needs more changes, and it puts the "hard" limit on > MAX_URETPROBE_DEPTH. Plus it can not solve another problem partially > fixed by the next patch. > > Note: this change has no effect on !x86, the arch-agnostic version of > arch_uretprobe_is_alive() just returns "true". > > TODO: as documented by the previous change, arch_uretprobe_is_alive() > can be fooled by sigaltstack/etc. > > Signed-off-by: Oleg Nesterov <o...@redhat.com> Acked-by: Anton Arapov <ara...@gmail.com>
> --- > kernel/events/uprobes.c | 29 ++++++++++++++++++----------- > 1 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index c5f316e..93d939c 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -1774,6 +1774,7 @@ static void handle_trampoline(struct pt_regs *regs) > { > struct uprobe_task *utask; > struct return_instance *ri, *next; > + bool valid; > > utask = current->utask; > if (!utask) > @@ -1783,18 +1784,24 @@ static void handle_trampoline(struct pt_regs *regs) > if (!ri) > goto sigill; > > - next = find_next_ret_chain(ri); > - /* > - * TODO: we should throw out return_instance's invalidated by > - * longjmp(), currently we assume that the probed function always > - * returns. > - */ > - instruction_pointer_set(regs, ri->orig_ret_vaddr); > do { > - handle_uretprobe_chain(ri, regs); > - ri = free_ret_instance(ri); > - utask->depth--; > - } while (ri != next); > + /* > + * We should throw out the frames invalidated by longjmp(). > + * If this chain is valid, then the next one should be alive > + * or NULL; the latter case means that nobody but ri->func > + * could hit this trampoline on return. TODO: sigaltstack(). > + */ > + next = find_next_ret_chain(ri); > + valid = !next || arch_uretprobe_is_alive(next, regs); > + > + instruction_pointer_set(regs, ri->orig_ret_vaddr); > + do { > + if (valid) > + handle_uretprobe_chain(ri, regs); > + ri = free_ret_instance(ri); > + utask->depth--; > + } while (ri != next); > + } while (!valid); > > utask->return_instances = ri; > return; > -- > 1.5.5.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/