[PATCH] uprobes: Allocate ->utask before handler_chain() for tracing handlers

[Oleg Nesterov <oleg@redhat.com>]

Hi Namhyung,

On 11/25, Namhyung Kim wrote:
>
> 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 realized that current->utask is not set

Aaah, I am stipid. Yes, I forgot that it is always NULL until the
task does xol or prepare_uretprobe() for the 1st time.... And on
powerpc it can be always NULL because it can likely emulate the
probed insn.

> or has
> an invalid vaddr when handler_chain() is called.

But this should not matter at all? you should not rely on the value of
->vaddr, you should use at as uninitialized scratchpad. And in fact we
were going to add another member into the union which should be used
instead later (but lets ignore this for now).

> So I had to apply
> following patch and it seems to work well for me.  Could you confirm it?

I don't think we need this patch, see below.

> @@ -1744,11 +1744,17 @@ static void handle_swbp(struct pt_regs *regs)
>  	if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags)))
>  		goto out;
>
> +	utask = get_utask();

Yes, we need this until we find another way to pass the additional info
to ->fetch() methods. This is a bit unfortunate, may be we fill find a
better solution later.

But until then we can probably tolerate the hack below, what do you
think?

Oleg.
---

Subject: [PATCH] uprobes: Allocate ->utask before handler_chain() for tracing handlers

uprobe_trace_print() and uprobe_perf_print() need to pass the additional
info to call_fetch() methods, currently there is no simple way to do this.

current->utask looks like a natural place to hold this info, but we need
to allocate it before handler_chain().

This is a bit unfortunate, perhaps we will find a better solution later,
but this is simple and should work right now.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/events/uprobes.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index b886a5e..307d87c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1854,6 +1854,10 @@ static void handle_swbp(struct pt_regs *regs)
 	if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags)))
 		goto out;
 
+	/* Tracing handlers use ->utask to communicate with fetch methods */
+	if (!get_utask())
+		goto out;
+
 	handler_chain(uprobe, regs);
 	if (can_skip_sstep(uprobe, regs))
 		goto out;
-- 
1.5.5.1