Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-24 Thread Namhyung Kim
Hi Oleg, On Tue, 12 Nov 2013 17:00:01 +0900, Namhyung Kim wrote: > For @+addr syntax: user-space uses relative symbol address from a loaded >base address and kernel calculates the base address >using "current->utask->vaddr - tu->offset". I tried this

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-24 Thread Namhyung Kim
Hi Oleg, On Tue, 12 Nov 2013 17:00:01 +0900, Namhyung Kim wrote: For @+addr syntax: user-space uses relative symbol address from a loaded base address and kernel calculates the base address using current-utask-vaddr - tu-offset. I tried this approach and

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-12 Thread Oleg Nesterov
Hi Namhyung, On 11/12, Namhyung Kim wrote: > > Let me clarify what I understand. > > For @addr syntax: kernel does no translation and uses given address Yes, > For @+addr syntax: user-space uses relative symbol address from a loaded >base address and kernel calculates the

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-12 Thread Namhyung Kim
Hi Oleg and Masami, On Sat, 9 Nov 2013 16:23:13 +0100, Oleg Nesterov wrote: > On 11/09, Masami Hiramatsu wrote: >> >> In that case, I suggest you to use "@+addr" for the relative address, >> since that is an offset, isn't that? :) > > Agreed, @+addr looks better! Looks good to me too. > >> BTW,

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-12 Thread Namhyung Kim
Hi Oleg and Masami, On Sat, 9 Nov 2013 16:23:13 +0100, Oleg Nesterov wrote: On 11/09, Masami Hiramatsu wrote: In that case, I suggest you to use @+addr for the relative address, since that is an offset, isn't that? :) Agreed, @+addr looks better! Looks good to me too. BTW, it seems that

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-12 Thread Oleg Nesterov
Hi Namhyung, On 11/12, Namhyung Kim wrote: Let me clarify what I understand. For @addr syntax: kernel does no translation and uses given address Yes, For @+addr syntax: user-space uses relative symbol address from a loaded base address and kernel calculates the base

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-11 Thread Namhyung Kim
Hi Oleg, On Fri, 8 Nov 2013 18:00:17 +0100, Oleg Nesterov wrote: > On 11/07, Namhyung Kim wrote: >> >> On Wed, 6 Nov 2013 19:24:08 +0100, Oleg Nesterov wrote: >> > >> > Note that instruction_pointer_set() is not really nice in any case, >> > this can obviously confuse FETCH_MTD_reg("ip"). >> >>

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-11 Thread Namhyung Kim
Hi Oleg, On Fri, 8 Nov 2013 18:00:17 +0100, Oleg Nesterov wrote: On 11/07, Namhyung Kim wrote: On Wed, 6 Nov 2013 19:24:08 +0100, Oleg Nesterov wrote: Note that instruction_pointer_set() is not really nice in any case, this can obviously confuse FETCH_MTD_reg(ip). Yes. But lets

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-09 Thread Oleg Nesterov
On 11/09, Masami Hiramatsu wrote: > > In that case, I suggest you to use "@+addr" for the relative address, > since that is an offset, isn't that? :) Agreed, @+addr looks better! > BTW, it seems that @addr syntax is hard to use for uprobes, because > current uprobes is based on a binary, not a

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-09 Thread Oleg Nesterov
On 11/09, Masami Hiramatsu wrote: In that case, I suggest you to use @+addr for the relative address, since that is an offset, isn't that? :) Agreed, @+addr looks better! BTW, it seems that @addr syntax is hard to use for uprobes, because current uprobes is based on a binary, not a process,

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-08 Thread Masami Hiramatsu
(2013/11/07 17:48), Namhyung Kim wrote: > On Wed, 6 Nov 2013 18:37:54 +0100, Oleg Nesterov wrote: >> On 11/06, Namhyung Kim wrote: >>> >>> On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: On 11/05, Oleg Nesterov wrote: > > As for "-= tu->offset"... Can't we avoid it? User-space

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-08 Thread Oleg Nesterov
On 11/07, Namhyung Kim wrote: > > On Wed, 6 Nov 2013 19:24:08 +0100, Oleg Nesterov wrote: > > > > Note that instruction_pointer_set() is not really nice in any case, > > this can obviously confuse FETCH_MTD_reg("ip"). > > Yes. > > > > > But lets ignore this. The solution is simple, we can pass/use

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-08 Thread Oleg Nesterov
Hi Namhyung, sorry for delay. On 11/07, Namhyung Kim wrote: > > >> > 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 ;) >

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-08 Thread Oleg Nesterov
Hi Namhyung, sorry for delay. On 11/07, Namhyung Kim wrote: 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

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-08 Thread Oleg Nesterov
On 11/07, Namhyung Kim wrote: On Wed, 6 Nov 2013 19:24:08 +0100, Oleg Nesterov wrote: Note that instruction_pointer_set() is not really nice in any case, this can obviously confuse FETCH_MTD_reg(ip). Yes. But lets ignore this. The solution is simple, we can pass/use this info via

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-08 Thread Masami Hiramatsu
(2013/11/07 17:48), Namhyung Kim wrote: On Wed, 6 Nov 2013 18:37:54 +0100, Oleg Nesterov wrote: On 11/06, Namhyung Kim wrote: On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: On 11/05, Oleg Nesterov wrote: As for -= tu-offset... Can't we avoid it? User-space needs to calculate the @

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-07 Thread Namhyung Kim
On Wed, 6 Nov 2013 19:24:08 +0100, Oleg Nesterov wrote: > Forgot to mention, > > On 11/06, Oleg Nesterov wrote: >> >> I meant, >> >> saved_ip = instruction_pointer(regs); >> >> // pass the "ip" which was used to calculate >> // the @addr argument to fetch_*()

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-07 Thread Namhyung Kim
On Wed, 6 Nov 2013 18:37:54 +0100, Oleg Nesterov wrote: > On 11/06, Namhyung Kim wrote: >> >> On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: >> > On 11/05, Oleg Nesterov wrote: >> >> >> >> As for "-= tu->offset"... Can't we avoid it? User-space needs to calculate >> >> the "@" argument

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-07 Thread Namhyung Kim
On Wed, 6 Nov 2013 18:37:54 +0100, Oleg Nesterov wrote: On 11/06, Namhyung Kim wrote: On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: On 11/05, Oleg Nesterov wrote: As for -= tu-offset... Can't we avoid it? User-space needs to calculate the @ argument anyway, why it can't also

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-07 Thread Namhyung Kim
On Wed, 6 Nov 2013 19:24:08 +0100, Oleg Nesterov wrote: Forgot to mention, On 11/06, Oleg Nesterov wrote: I meant, saved_ip = instruction_pointer(regs); // pass the ip which was used to calculate // the @addr argument to fetch_*() methods

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
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

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Oleg Nesterov
Forgot to mention, On 11/06, Oleg Nesterov wrote: > > I meant, > > saved_ip = instruction_pointer(regs); > > // pass the "ip" which was used to calculate > // the @addr argument to fetch_*() methods > > temp_ip = is_ret_probe(tu) ? func :

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Oleg Nesterov
On 11/06, Namhyung Kim wrote: > > On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: > > On 11/05, Oleg Nesterov wrote: > >> > >> 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? > >> > >> Or

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Oleg Nesterov
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

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: > On 11/05, Oleg Nesterov wrote: >> >> 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? >> >> Or perhaps we can change parse_probe_arg("@") to

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
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

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
On Tue, 5 Nov 2013 17:41:02 +0100, Oleg Nesterov wrote: > On 11/05, Namhyung Kim wrote: >> >> On Mon, 4 Nov 2013 19:57:54 +0100, Oleg Nesterov wrote: >> >> >> >> static void __user *get_user_vaddr(struct pt_regs *regs, unsigned long >> >> addr) >> >> { >> >> return (void __force

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
On Tue, 5 Nov 2013 17:33:41 +0100, Oleg Nesterov wrote: > On 11/05, Namhyung Kim wrote: >> >> On Mon, 4 Nov 2013 17:22:29 +0100, Oleg Nesterov wrote: >> > >> > So probably I was wrong and this all needs more thinking. Damn. >> >> :) > > Don't laugh at me ;) :) (And now I'm feeling guilty..) >

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
Hi Oleg, On Tue, 5 Nov 2013 17:28:20 +0100, Oleg Nesterov wrote: > On 11/05, Namhyung Kim wrote: >> >> On Mon, 4 Nov 2013 16:01:12 +0100, Oleg Nesterov wrote: >> > Or the syntax should be "name=probe @file/addr" or something like this. >> >> Okay. Let's call this kind of thing "cross-fetch" (or

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
Hi Oleg, On Tue, 5 Nov 2013 17:28:20 +0100, Oleg Nesterov wrote: On 11/05, Namhyung Kim wrote: On Mon, 4 Nov 2013 16:01:12 +0100, Oleg Nesterov wrote: Or the syntax should be name=probe @file/addr or something like this. Okay. Let's call this kind of thing cross-fetch (or a better name

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
On Tue, 5 Nov 2013 17:33:41 +0100, Oleg Nesterov wrote: On 11/05, Namhyung Kim wrote: On Mon, 4 Nov 2013 17:22:29 +0100, Oleg Nesterov wrote: So probably I was wrong and this all needs more thinking. Damn. :) Don't laugh at me ;) :) (And now I'm feeling guilty..) Perhaps we really

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
On Tue, 5 Nov 2013 17:41:02 +0100, Oleg Nesterov wrote: On 11/05, Namhyung Kim wrote: On Mon, 4 Nov 2013 19:57:54 +0100, Oleg Nesterov wrote: static void __user *get_user_vaddr(struct pt_regs *regs, unsigned long addr) { return (void __force __user *)addr +

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
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;

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: On 11/05, Oleg Nesterov wrote: 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? Or perhaps we can change parse_probe_arg(@) to update param ? Yes,

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Oleg Nesterov
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) {

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Oleg Nesterov
On 11/06, Namhyung Kim wrote: On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: On 11/05, Oleg Nesterov wrote: 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? Or perhaps we can change

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Oleg Nesterov
Forgot to mention, On 11/06, Oleg Nesterov wrote: I meant, saved_ip = instruction_pointer(regs); // pass the ip which was used to calculate // the @addr argument to fetch_*() methods temp_ip = is_ret_probe(tu) ? func : saved_ip;

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-06 Thread Namhyung Kim
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

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Oleg Nesterov wrote: > > 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? > > Or perhaps we can change parse_probe_arg("@") to update "param" ? Yes, > in this case it needs another argument, not

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
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 =

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Namhyung Kim wrote: > > On Mon, 4 Nov 2013 16:01:12 +0100, Oleg Nesterov wrote: > > Or the syntax should be "name=probe @file/addr" or something like this. > > Okay. Let's call this kind of thing "cross-fetch" (or a better name can > be suggested). Yes ;) and I am afraid there was some

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Namhyung Kim wrote: > > On Mon, 4 Nov 2013 17:22:29 +0100, Oleg Nesterov wrote: > > > > So probably I was wrong and this all needs more thinking. Damn. > > :) Don't laugh at me ;) > > Perhaps we really need to pass @file/offset, but it is not clear what > > we can do with

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Namhyung Kim wrote: > > On Mon, 4 Nov 2013 19:57:54 +0100, Oleg Nesterov wrote: > >> > >>static void __user *get_user_vaddr(struct pt_regs *regs, unsigned long > >> addr) > >>{ > >>return (void __force __user *)addr + instruction_pointer(regs); > >>} > >> > >> ?

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Namhyung Kim wrote: On Mon, 4 Nov 2013 19:57:54 +0100, Oleg Nesterov wrote: static void __user *get_user_vaddr(struct pt_regs *regs, unsigned long addr) { return (void __force __user *)addr + instruction_pointer(regs); } ? This should solve the

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Namhyung Kim wrote: On Mon, 4 Nov 2013 17:22:29 +0100, Oleg Nesterov wrote: So probably I was wrong and this all needs more thinking. Damn. :) Don't laugh at me ;) Perhaps we really need to pass @file/offset, but it is not clear what we can do with bss/anon-mapping. The

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Namhyung Kim wrote: On Mon, 4 Nov 2013 16:01:12 +0100, Oleg Nesterov wrote: Or the syntax should be name=probe @file/addr or something like this. Okay. Let's call this kind of thing cross-fetch (or a better name can be suggested). Yes ;) and I am afraid there was some confusion

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
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 =

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-05 Thread Oleg Nesterov
On 11/05, Oleg Nesterov wrote: 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? Or perhaps we can change parse_probe_arg(@) to update param ? Yes, in this case it needs another argument, not sure... Or,

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
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;

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 19:57:54 +0100, Oleg Nesterov wrote: > On 11/04, Oleg Nesterov wrote: >> >> On 11/04, Oleg Nesterov wrote: >> > >> > On 11/04, Oleg Nesterov wrote: >> > > >> > > But in any case, I strongly believe that it doesn't make any sense to >> > > rely on tu->inode in get_user_vaddr().

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 19:47:41 +0100, Oleg Nesterov wrote: > On 11/04, Oleg Nesterov wrote: >> >> On 11/04, Oleg Nesterov wrote: >> > >> > But in any case, I strongly believe that it doesn't make any sense to >> > rely on tu->inode in get_user_vaddr(). >> >> Hmm. But I forgot about the case when you

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 17:22:29 +0100, Oleg Nesterov wrote: > On 11/04, Oleg Nesterov wrote: >> >> But in any case, I strongly believe that it doesn't make any sense to >> rely on tu->inode in get_user_vaddr(). > > Hmm. But I forgot about the case when you probe the function in libc > and want to dump

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 16:51:31 +0100, Oleg Nesterov wrote: > On 11/04, Namhyung Kim wrote: >> On Mon, 04 Nov 2013 17:46:41 +0900, Namhyung Kim wrote: >> > On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: >> >> - this only allows to read the data from the same binary. >> > >> > Right. This is

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 16:01:12 +0100, Oleg Nesterov wrote: > On 11/04, Namhyung Kim wrote: >> >> On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: >> > >> > This does not look right to me. >> > >> > - get_user_vaddr() is costly, it does vma_interval_tree_foreach() under >> > ->i_mmap_mutex.

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Oleg Nesterov wrote: > > On 11/04, Oleg Nesterov wrote: > > > > On 11/04, Oleg Nesterov wrote: > > > > > > But in any case, I strongly believe that it doesn't make any sense to > > > rely on tu->inode in get_user_vaddr(). > > > > Hmm. But I forgot about the case when you probe the

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Oleg Nesterov wrote: > > On 11/04, Oleg Nesterov wrote: > > > > But in any case, I strongly believe that it doesn't make any sense to > > rely on tu->inode in get_user_vaddr(). > > Hmm. But I forgot about the case when you probe the function in libc > and want to dump the variable in

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Oleg Nesterov wrote: > > But in any case, I strongly believe that it doesn't make any sense to > rely on tu->inode in get_user_vaddr(). Hmm. But I forgot about the case when you probe the function in libc and want to dump the variable in libc... So probably I was wrong and this all

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Namhyung Kim wrote: > On Mon, 04 Nov 2013 17:46:41 +0900, Namhyung Kim wrote: > > On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: > >> - this only allows to read the data from the same binary. > > > > Right. This is also an unnecessary restriction. We should be able to > >

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Namhyung Kim wrote: > > On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: > > > > This does not look right to me. > > > > - get_user_vaddr() is costly, it does vma_interval_tree_foreach() under > > ->i_mmap_mutex. > > Hmm.. yes, I think this is not needed. I guess it should

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 04 Nov 2013 17:46:41 +0900, Namhyung Kim wrote: > On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: >> - this only allows to read the data from the same binary. > > Right. This is also an unnecessary restriction. We should be able to > access data in other binary. Hmm.. I guess

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: > Hello, > > Let me first apologize again if this was already discussed. And I also > need to mention that I know almost nothing about elf/randomization/etc. No no, this was not discussed enough. And I really appreciate your thorough

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Oleg Nesterov wrote: But in any case, I strongly believe that it doesn't make any sense to rely on tu-inode in get_user_vaddr(). Hmm. But I forgot about the case when you probe the function in libc and want to dump the variable in libc... So probably I was wrong and this all needs

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: But in any case, I strongly believe that it doesn't make any sense to rely on tu-inode in get_user_vaddr(). Hmm. But I forgot about the case when you probe the function in libc and want to dump the variable in libc... So

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: But in any case, I strongly believe that it doesn't make any sense to rely on tu-inode in get_user_vaddr(). Hmm. But I forgot about the case when you probe the function in libc and

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 16:01:12 +0100, Oleg Nesterov wrote: On 11/04, Namhyung Kim wrote: On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: This does not look right to me. - get_user_vaddr() is costly, it does vma_interval_tree_foreach() under -i_mmap_mutex. Hmm.. yes, I think

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 16:51:31 +0100, Oleg Nesterov wrote: On 11/04, Namhyung Kim wrote: On Mon, 04 Nov 2013 17:46:41 +0900, Namhyung Kim wrote: On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: - this only allows to read the data from the same binary. Right. This is also an

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 17:22:29 +0100, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: But in any case, I strongly believe that it doesn't make any sense to rely on tu-inode in get_user_vaddr(). Hmm. But I forgot about the case when you probe the function in libc and want to dump the

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 19:47:41 +0100, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: But in any case, I strongly believe that it doesn't make any sense to rely on tu-inode in get_user_vaddr(). Hmm. But I forgot about the case when you probe the function

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 4 Nov 2013 19:57:54 +0100, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: On 11/04, Oleg Nesterov wrote: But in any case, I strongly believe that it doesn't make any sense to rely on tu-inode in get_user_vaddr(). Hmm. But I forgot

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
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;

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: Hello, Let me first apologize again if this was already discussed. And I also need to mention that I know almost nothing about elf/randomization/etc. No no, this was not discussed enough. And I really appreciate your thorough review! :)

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Namhyung Kim
On Mon, 04 Nov 2013 17:46:41 +0900, Namhyung Kim wrote: On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: - this only allows to read the data from the same binary. Right. This is also an unnecessary restriction. We should be able to access data in other binary. Hmm.. I guess this

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Namhyung Kim wrote: On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: This does not look right to me. - get_user_vaddr() is costly, it does vma_interval_tree_foreach() under -i_mmap_mutex. Hmm.. yes, I think this is not needed. I guess it should lookup a proper

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-04 Thread Oleg Nesterov
On 11/04, Namhyung Kim wrote: On Mon, 04 Nov 2013 17:46:41 +0900, Namhyung Kim wrote: On Sat, 2 Nov 2013 16:54:58 +0100, Oleg Nesterov wrote: - this only allows to read the data from the same binary. Right. This is also an unnecessary restriction. We should be able to access data in

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-02 Thread Oleg Nesterov
Hello, Let me first apologize again if this was already discussed. And I also need to mention that I know almost nothing about elf/randomization/etc. However, On 10/29, Namhyung Kim wrote: > > # nm foo | grep -e glob$ -e str -e foo > 006008bc D foo > 006008a8 D glob >

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-11-02 Thread Oleg Nesterov
Hello, Let me first apologize again if this was already discussed. And I also need to mention that I know almost nothing about elf/randomization/etc. However, On 10/29, Namhyung Kim wrote: # nm foo | grep -e glob$ -e str -e foo 006008bc D foo 006008a8 D glob

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-10-30 Thread Masami Hiramatsu
(2013/10/29 15:53), Namhyung Kim wrote: > Hello, > > This patchset implements memory (address), stack[N], deference, > bitfield and retval (it needs uretprobe tho) fetch methods for > uprobes. It's based on the previous work [1] done by Hyeoncheol Lee. > > Now kprobes and uprobes have their own

Re: [PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-10-30 Thread Masami Hiramatsu
(2013/10/29 15:53), Namhyung Kim wrote: Hello, This patchset implements memory (address), stack[N], deference, bitfield and retval (it needs uretprobe tho) fetch methods for uprobes. It's based on the previous work [1] done by Hyeoncheol Lee. Now kprobes and uprobes have their own

[PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-10-29 Thread Namhyung Kim
Hello, This patchset implements memory (address), stack[N], deference, bitfield and retval (it needs uretprobe tho) fetch methods for uprobes. It's based on the previous work [1] done by Hyeoncheol Lee. Now kprobes and uprobes have their own fetch_type_tables and, in turn, memory and stack

[PATCHSET 00/13] tracing/uprobes: Add support for more fetch methods (v6)

2013-10-29 Thread Namhyung Kim
Hello, This patchset implements memory (address), stack[N], deference, bitfield and retval (it needs uretprobe tho) fetch methods for uprobes. It's based on the previous work [1] done by Hyeoncheol Lee. Now kprobes and uprobes have their own fetch_type_tables and, in turn, memory and stack