Hi Oleg, On Wed, 6 Nov 2013 17:28:06 +0100, Oleg Nesterov wrote: > 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.
Okay. > >> 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. Right. > > 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. Agreed. >> > 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. That said, I'd prefer the original "-= -tu->offset" approach. It'll make debugging easier IMHO. > >> >> 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). Ah, okay. > >> 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(). Ah, I was confused that the 'func' is somewhat available in trace_uprobe and it can make to avoid passing regs to get_user_vaddr(). Sorry for the noise. > See also another email... Will do. Thanks, Namhyung -- 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/