Messages in this thread |  | | Date | Wed, 6 Nov 2013 17:28:06 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6) |
| |
On 11/06, Namhyung Kim wrote: > > On Tue, 5 Nov 2013 18:45:35 +0100, Oleg Nesterov wrote: > > On 11/05, Namhyung Kim wrote: > >> > >> This is what I have for now: > >> > >> static void __user *get_user_vaddr(struct pt_regs *regs, unsigned long addr, > >> struct trace_uprobe *tu) > >> { > >> unsigned long base_addr; > >> unsigned long vaddr; > >> > >> base_addr = instruction_pointer(regs) - tu->offset; > >> vaddr = base_addr + addr; > >> > >> return (void __force __user *) vaddr; > >> } > >> > >> When I tested it, it was able to fetch global and bss data from both of > >> executable and library properly. > > > > Heh ;) I didn't expect you will agree with this suggestion. But if you > > think it can work - great! > > It seems to work for me well except the cross-fetch.
Yes, but cross-fetching needs something different anyway, so I think we should discuss this separately.
> But I'm not sure it'll work for every cases.
I think "ip - tu->offset + vaddr" trick should always work, just we need to calculate this "vaddr" passed as an argument correctly.
Except: user-space can create another executable mapping and call the probed function via another address, but I think we can ignore this. And I think we can do nothing in this case, because in this case we can't even rely on tu->inode.
But,
> It would be great if some > elf gurus come up and give some feedbacks. > > Masami?
Yes.
> > As for "-= tu->offset"... Can't we avoid it? User-space needs to calculate > > the "@" argument anyway, why it can't also substruct this offset? > > Hmm.. it makes sense too. :)
I am no longer sure ;)
This way the "@" argument will look more confusing, it will depend on the address/offset of the probed insn. But again, I do not know, this is up to you.
> >> But it still doesn't work for uretprobes > >> as you said before. > > > > This looks simple, > > > > + if (is_ret_probe(tu)) { > > + saved_ip = instruction_pointer(regs); > > + instruction_pointer_set(func); > > + } > > store_trace_args(...); > > + if (is_ret_probe(tu)) > > + instruction_pointer_set(saved_ip); > > > > although not pretty. > > So for normal non-uretprobes, func == instruction_pointer(), right?
No, for normal non-uretprobes func == 0 (actually, undefined).
> If so, just passing func as you suggested looks better than this.
Not sure I understand... OK, we can change uprobe_trace_func() and uprobe_perf_func()
if (!is_ret_probe(tu)) - uprobe_trace_print(tu, 0, regs); + uprobe_trace_print(tu, instruction_pointer(regs), regs); return 0;
but why?
We need the "saved_ip" ugly hack above only if is_ret_probe() == T and thus instruction_pointer() doesn't match the address of the probed function. And there is no way to pass some additional info to call_fetch/etc from uprobe_*_print().
See also another email...
Oleg.
|  |