lkml.org 
[lkml]   [2015]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v4 2/2] perf: report/annotate: fix segfault problem.
    Date
    perf report and perf annotate are easy to trigger segfault if trace data
    contain kernel module information like this:

    # perf report -D -i ./perf.data
    ...
    0 0 0x188 [0x50]: PERF_RECORD_MMAP -1/0: [0xffffffbff1018000(0xf068000) @ 0]: x [test_module]
    ...

    # perf report -i ./perf.data --objdump=/path/to/objdump --kallsyms=/path/to/kallsyms

    perf: Segmentation fault
    -------- backtrace --------
    /path/to/perf[0x503478]
    /lib64/libc.so.6(+0x3545f)[0x7fb201f3745f]
    /path/to/perf[0x499b56]
    /path/to/perf(dso__load_kallsyms+0x13c)[0x49b56c]
    /path/to/perf(dso__load+0x72e)[0x49c21e]
    /path/to/perf(map__load+0x6e)[0x4ae9ee]
    /path/to/perf(thread__find_addr_map+0x24c)[0x47deec]
    /path/to/perf(perf_event__preprocess_sample+0x88)[0x47e238]
    /path/to/perf[0x43ad02]
    /path/to/perf[0x4b55bc]
    /path/to/perf(ordered_events__flush+0xca)[0x4b57ea]
    /path/to/perf[0x4b1a01]
    /path/to/perf(perf_session__process_events+0x3be)[0x4b428e]
    /path/to/perf(cmd_report+0xf11)[0x43bfc1]
    /path/to/perf[0x474702]
    /path/to/perf(main+0x5f5)[0x42de95]
    /lib64/libc.so.6(__libc_start_main+0xf4)[0x7fb201f23bd4]
    /path/to/perf[0x42dfc4]

    This is because __kmod_path__parse regard '[' leading name as kernel
    instead of kernel module. The DSO will then be passed to
    dso__load_kernel_sym() then dso__load_kcore() because of --kallsyms
    argument. The segfault is triggered because the kmap structure is not
    initialized.

    Although in --vmlinux case such segfault can be avoided, the symbols in
    the kernel module are unable to be retrived since the attribute of DSO
    is incorrect.

    This patch fixes __kmod_path__parse, make it regard names like
    '[test_module]' as kernel module.

    According to suggestion from Arnaldo Carvalho de Melo (
    https://lkml.org/lkml/2015/4/6/90), appends cpumode argument to
    __kmod_path__parse() to ensure the passed path is a kernel mmap.

    kmod-path.c is also update to reflect the above changes.

    Signed-off-by: Wang Nan <wangnan0@huawei.com>
    ---

    Different from v3:

    Don't warn if cpumode is unknown.

    ---

    tools/perf/tests/kmod-path.c | 43 +++++++++++++++++++++++++++++++----
    tools/perf/util/dso.c | 54 +++++++++++++++++++++++++++++++++++++++-----
    tools/perf/util/dso.h | 12 ++++++----
    tools/perf/util/header.c | 2 +-
    tools/perf/util/machine.c | 4 +++-
    5 files changed, 98 insertions(+), 17 deletions(-)

    diff --git a/tools/perf/tests/kmod-path.c b/tools/perf/tests/kmod-path.c
    index e8d7cbb..5d57df6 100644
    --- a/tools/perf/tests/kmod-path.c
    +++ b/tools/perf/tests/kmod-path.c
    @@ -4,14 +4,14 @@
    #include "debug.h"

    static int test(const char *path, bool alloc_name, bool alloc_ext,
    - bool kmod, bool comp, const char *name, const char *ext)
    + bool kmod, bool comp, const char *name, const char *ext, int cpumode)
    {
    struct kmod_path m;

    memset(&m, 0x0, sizeof(m));

    TEST_ASSERT_VAL("kmod_path__parse",
    - !__kmod_path__parse(&m, path, alloc_name, alloc_ext));
    + !__kmod_path__parse(&m, path, cpumode, alloc_name, alloc_ext));

    pr_debug("%s - alloc name %d, alloc ext %d, kmod %d, comp %d, name '%s', ext '%s'\n",
    path, alloc_name, alloc_ext, m.kmod, m.comp, m.name, m.ext);
    @@ -34,8 +34,14 @@ static int test(const char *path, bool alloc_name, bool alloc_ext,
    return 0;
    }

    -#define T(path, an, ae, k, c, n, e) \
    - TEST_ASSERT_VAL("failed", !test(path, an, ae, k, c, n, e))
    +
    +#define _T(path, an, ae, k, c, n, e, m) \
    + TEST_ASSERT_VAL("failed", !test(path, an, ae, k, c, n, e, m))
    +
    +#define T(path, an, ae, k, c, n, e) _T(path, an, ae, k, c, n, e, \
    + PERF_RECORD_MISC_CPUMODE_UNKNOWN)
    +#define TU(path, an, ae, k, c, n, e) _T(path, an, ae, k, c, n, e, \
    + PERF_RECORD_MISC_USER)

    int test__kmod_path__parse(void)
    {
    @@ -69,5 +75,34 @@ int test__kmod_path__parse(void)
    T("x.ko.gz", true , false , true, true, "[x]", NULL);
    T("x.ko.gz", false , false , true, true, NULL , NULL);

    + /* path alloc_name alloc_ext kmod comp name ext */
    + T("[test_module]", true , true , true, false, "[test_module]" , NULL);
    + T("[test_module]", false , true , true, false, NULL , NULL);
    + T("[test_module]", true , false , true, false, "[test_module]" , NULL);
    + T("[test_module]", false , false , true, false, NULL , NULL);
    +
    + /* path alloc_name alloc_ext kmod comp name ext */
    + T("[test.module]", true , true , true, false, "[test.module]" , NULL);
    + T("[test.module]", false , true , true, false, NULL , NULL);
    + T("[test.module]", true , false , true, false, "[test.module]" , NULL);
    + T("[test.module]", false , false , true, false, NULL , NULL);
    +
    + /* path alloc_name alloc_ext kmod comp name ext */
    + TU("[vdso]", true , true , false, false, "[vdso]" , NULL);
    + TU("[vdso]", false , true , false, false, NULL , NULL);
    + TU("[vdso]", true , false , false, false, "[vdso]" , NULL);
    + TU("[vdso]", false , false , false, false, NULL , NULL);
    +
    + /* path alloc_name alloc_ext kmod comp name ext */
    + TU("[vsyscall]", true , true , false, false, "[vsyscall]" , NULL);
    + TU("[vsyscall]", false , true , false, false, NULL , NULL);
    + TU("[vsyscall]", true , false , false, false, "[vsyscall]" , NULL);
    + TU("[vsyscall]", false , false , false, false, NULL , NULL);
    +
    + /* path alloc_name alloc_ext kmod comp name ext */
    + T("[kernel.kallsyms]", true , true , false, false, "[kernel.kallsyms]" , NULL);
    + T("[kernel.kallsyms]", false , true , false, false, NULL , NULL);
    + T("[kernel.kallsyms]", true , false , false, false, "[kernel.kallsyms]" , NULL);
    + T("[kernel.kallsyms]", false , false , false, false, NULL , NULL);
    return 0;
    }
    diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
    index fc0ddd5..875f7c9 100644
    --- a/tools/perf/util/dso.c
    +++ b/tools/perf/util/dso.c
    @@ -165,11 +165,11 @@ bool is_supported_compression(const char *ext)
    return false;
    }

    -bool is_kernel_module(const char *pathname)
    +bool is_kernel_module(const char *pathname, int cpumode)
    {
    struct kmod_path m;

    - if (kmod_path__parse(&m, pathname))
    + if (kmod_path__parse(&m, pathname, cpumode))
    return NULL;

    return m.kmod;
    @@ -210,21 +210,56 @@ bool dso__needs_decompress(struct dso *dso)
    * Returns 0 if there's no strdup error, -ENOMEM otherwise.
    */
    int __kmod_path__parse(struct kmod_path *m, const char *path,
    - bool alloc_name, bool alloc_ext)
    + int cpumode, bool alloc_name, bool alloc_ext)
    {
    const char *name = strrchr(path, '/');
    const char *ext = strrchr(path, '.');
    + bool is_simple_name = false;
    + bool cpu_mode_kernel, cpu_mode_unknown = false, is_kernel = false;
    +
    + /* treat PERF_RECORD_MISC_CPUMODE_UNKNOWN as kernel. */
    + switch (cpumode & PERF_RECORD_MISC_CPUMODE_MASK) {
    + case PERF_RECORD_MISC_USER:
    + case PERF_RECORD_MISC_HYPERVISOR:
    + case PERF_RECORD_MISC_GUEST_USER:
    + cpu_mode_kernel = false;
    + break;
    + case PERF_RECORD_MISC_CPUMODE_UNKNOWN:
    + cpu_mode_unknown = true;
    + /* fall through */
    + default:
    + cpu_mode_kernel = true;
    + }

    memset(m, 0x0, sizeof(*m));
    name = name ? name + 1 : path;

    + /*
    + * '.' is also a valid character. For example: [aaa.bbb] is a
    + * valid module name. '[' should have higher priority than
    + * '.ko' suffix.
    + *
    + * The kernel names are from machine__mmap_name. Such
    + * name should belong to kernel itself, not kernel module.
    + */
    + if (name[0] == '[') {
    + is_simple_name = true;
    + if ((strncmp(name, "[kernel.kallsyms]", 17) == 0) ||
    + (strncmp(name, "[guest.kernel.kallsyms", 22) == 0)) {
    + m->kmod = false;
    + is_kernel = true;
    + } else
    + m->kmod = cpu_mode_kernel;
    + }
    +
    /* No extension, just return name. */
    - if (ext == NULL) {
    + if ((ext == NULL) || is_simple_name) {
    if (alloc_name) {
    m->name = strdup(name);
    - return m->name ? 0 : -ENOMEM;
    + if (!m->name)
    + return -ENOMEM;
    }
    - return 0;
    + goto out;
    }

    if (is_supported_compression(ext + 1)) {
    @@ -255,6 +290,13 @@ int __kmod_path__parse(struct kmod_path *m, const char *path,
    return -ENOMEM;
    }
    }
    +out:
    + if (!cpu_mode_unknown) {
    + if ((m->kmod && !cpu_mode_kernel) ||
    + (cpu_mode_kernel && !m->kmod && !is_kernel))
    + pr_warning("Kmod (%s) and cpumode (%d) inconsistent\n",
    + path, cpumode);
    + }

    return 0;
    }
    diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
    index e0901b4..0079b2b 100644
    --- a/tools/perf/util/dso.h
    +++ b/tools/perf/util/dso.h
    @@ -216,7 +216,7 @@ char dso__symtab_origin(const struct dso *dso);
    int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type type,
    char *root_dir, char *filename, size_t size);
    bool is_supported_compression(const char *ext);
    -bool is_kernel_module(const char *pathname);
    +bool is_kernel_module(const char *pathname, int cpumode);
    bool decompress_to_file(const char *ext, const char *filename, int output_fd);
    bool dso__needs_decompress(struct dso *dso);

    @@ -228,11 +228,13 @@ struct kmod_path {
    };

    int __kmod_path__parse(struct kmod_path *m, const char *path,
    - bool alloc_name, bool alloc_ext);
    + int cpumode, bool alloc_name, bool alloc_ext);

    -#define kmod_path__parse(__m, __p) __kmod_path__parse(__m, __p, false, false)
    -#define kmod_path__parse_name(__m, __p) __kmod_path__parse(__m, __p, true , false)
    -#define kmod_path__parse_ext(__m, __p) __kmod_path__parse(__m, __p, false, true)
    +#define kmod_path__parse(__m, __p, __c) __kmod_path__parse(__m, __p, __c, false, false)
    +#define kmod_path__parse_name(__m, __p) __kmod_path__parse(__m, __p, \
    + PERF_RECORD_MISC_CPUMODE_UNKNOWN, true , false)
    +#define kmod_path__parse_ext(__m, __p) __kmod_path__parse(__m, __p, \
    + PERF_RECORD_MISC_CPUMODE_UNKNOWN, false, true)

    /*
    * The dso__data_* external interface provides following functions:
    diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
    index fb43215..d106e12 100644
    --- a/tools/perf/util/header.c
    +++ b/tools/perf/util/header.c
    @@ -1266,7 +1266,7 @@ static int __event_process_build_id(struct build_id_event *bev,

    dso__set_build_id(dso, &bev->build_id);

    - if (!is_kernel_module(filename))
    + if (!is_kernel_module(filename, misc))
    dso->kernel = dso_type;

    build_id__sprintf(dso->build_id, sizeof(dso->build_id),
    diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
    index e45c8f3..e084943 100644
    --- a/tools/perf/util/machine.c
    +++ b/tools/perf/util/machine.c
    @@ -1109,7 +1109,9 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
    struct dso *dso;

    list_for_each_entry(dso, &machine->kernel_dsos.head, node) {
    - if (is_kernel_module(dso->long_name))
    + if (is_kernel_module(dso->long_name,
    + event->header.misc &
    + PERF_RECORD_MISC_CPUMODE_MASK))
    continue;

    kernel = dso;
    --
    1.8.3.4


    \
     
     \ /
      Last update: 2015-04-08 06:21    [W:3.817 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site