lkml.org 
[lkml]   [2014]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] kprobes: add kprobe_is_function_probed()
    On Tue, 21 Oct 2014, Josh Poimboeuf wrote:

    > > This is a rather difficult call actually. I am of course aware of the fact
    > > that kernel fucntions can't be uniquely identified by name, but when
    > > thinking about this, one has to consider:
    > >
    > > - ftrace primary userspace interface (set_ftrace_filter) is based on
    > > function names
    > > - kprobe tracer and uprobe trace primary userspace interface
    > > (/sys/kernel/debug/tracing/kprobe_events and uprobe_events respectively)
    > > are primarily based on function names
    >
    > True, the user space interfaces are done by name, but the kernel
    > interfaces aren't necessarily so (see ftrace_set_filter_ip and struct
    > kprobe.addr). This is a kernel interface so we can be more precise.

    We could probably have both interfaces available (i.e. allowing lookup by
    name or by address range, and let the caller decide/handle the
    potential duplicates).

    > > - the number of conflicts is probably zero, or very close to zero. Try to
    > > run this
    > >
    > > cut -f3 -d' ' /proc/kallsyms | sort | uniq -c
    > >
    > > So it's questionable whether all the back and forth name->address->name
    > > translations all over the place are worth all the trouble.
    >
    > On my kernel:
    >
    > $ grep ' t \| T ' /proc/kallsyms | awk '{print $3}' |sort |uniq -d |wc -l
    > 395
    >
    > So there are at least 395 places where this could go wrong...

    This is broken anyway ... this will add up a lot of unrelated things
    together (umask_show, _iommu_cpumask_show, __uncore_umask_show,
    wq_cpumask_show, etc). I believe looking at

    cut -f3 -d' ' /proc/kallsyms | sort | uniq -c | sort -nr -k1

    gives slightly better picture.

    Also keep in mind that a *lot* of the hits are not functions.

    > With kpatch, we're setting the ftrace filter by address so we don't
    > patch the wrong function. So we already have the address. We also have
    > the function length, which is needed for our backtrace safety checks.
    >
    > I'm guessing kGraft doesn't have the address + length? I think you
    > could call kallsyms_lookup() to get both values.
    >
    > Maybe we should see what our unified live patching code ends up looking
    > like before deciding what interface(s) we need here?

    Yes, that probably makes sense indeed. I am talking to David Miller wrt.
    mailinglist creation on vger.kernel.org as we speak, hopefully it'll
    materialize soon.

    > > Do we need to be race-free here? If userspace is both instantiating new
    > > krpobe and initiating live kernel patching at the "same time", I don't
    > > think kernel should try to solve such race ... it's simply there by
    > > definition, depending on, for example, scheduling decisions.
    >
    > If we're not preventing it, but instead just printing a warning, then
    > yeah, that sounds fine to me.
    >
    > I think it would also be nice to do the reverse, i.e. have kprobes emit
    > a warning if someone tries to probe a replaced function.

    Indeed, good idea!

    Thanks,

    --
    Jiri Kosina
    SUSE Labs


    \
     
     \ /
      Last update: 2014-10-22 00:01    [W:3.074 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site