lkml.org 
[lkml]   [2013]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    Subject[PATCH] uprobes: Allocate ->utask before handler_chain() for tracing handlers
    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



    \
     
     \ /
      Last update: 2013-11-25 21:41    [W:4.628 / U:0.884 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site