All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Mark Wielaard <mjw@redhat.com>
Subject: Re: Re: perf-probe crash in dwarf_getcfi_elf
Date: Wed, 31 Dec 2014 07:13:31 +0900	[thread overview]
Message-ID: <54A3238B.90805@hitachi.com> (raw)
In-Reply-To: <20141230090533.GH6081@sejong>

(2014/12/30 18:05), Namhyung Kim wrote:
> On Tue, Dec 30, 2014 at 05:47:08PM +0900, Namhyung Kim wrote:
>> On Mon, Dec 29, 2014 at 09:39:18PM -0700, David Ahern wrote:
>>> Hi Namhyung:
>>>
>>> Using perf-probe from top of Linus' tree I get a segfault on both Fedora 16
>>> and 18 (does not crash on Fedora 20). Command used is:
>>>
>>> perf probe -x /lib64/libc-2.14.90.so -a 'malloc  size=%di'
>>>
>>> git bisect points to:
>>>
>>> commit 03d89412981a7681971bc77edba1669595763030
>>> Author: Namhyung Kim <namhyung@kernel.org>
>>> Date:   Mon Apr 7 16:05:48 2014 +0900
>>>
>>>     perf probe: Use dwarf_getcfi_elf() instead of dwarf_getcfi()
>>>
>>
>> It seems to be related to below commit in elfutils.  We might need to
>> check .eh_frame section has SHT_PROGBITS.  Will send a patch soon.
> 
> 
>>From f56964e74d60a9921214d0e2e5c3d082f5a910c1 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung@kernel.org>
> Date: Tue, 30 Dec 2014 17:47:47 +0900
> Subject: [PATCH] perf probe: Fix a segfault on old libdw
> 
> David reported that perf can segfault when adding an uprobe event like
> this:
> 
>   $ perf probe -x /lib64/libc-2.14.90.so -a 'malloc  size=%di'
> 
>   (gdb) bt
>   #0  parse_eh_frame_hdr (hdr=0x0, hdr_size=2596, hdr_vaddr=71788,
>       ehdr=0x7fffffffd390, eh_frame_vaddr=
>       0x7fffffffd378, table_entries=0x8808d8, table_encoding=0x8808e0 "") at
>       dwarf_getcfi_elf.c:79
>   #1  0x000000385f81615a in getcfi_scn_eh_frame (hdr_vaddr=71788,
>       hdr_scn=0x8839b0, shdr=0x7fffffffd2f0, scn=<optimized out>,
>       ehdr=0x7fffffffd390, elf=0x882b30) at dwarf_getcfi_elf.c:231
>   #2  getcfi_shdr (ehdr=0x7fffffffd390, elf=0x882b30) at dwarf_getcfi_elf.c:283
>   #3  dwarf_getcfi_elf (elf=0x882b30) at dwarf_getcfi_elf.c:309
>   #4  0x00000000004d5bac in debuginfo__find_probes (pf=0x7fffffffd4f0,
>       dbg=Unhandled dwarf expression opcode 0xfa) at util/probe-finder.c:993
>   #5  0x00000000004d634a in debuginfo__find_trace_events (dbg=0x880840,
>       pev=<optimized out>, tevs=0x880f88, max_tevs=<optimized out>) at
>       util/probe-finder.c:1200
>   #6  0x00000000004aed6b in try_to_find_probe_trace_events (target=0x881b20
>       "/lib64/libpthread-2.14.90.so",
>       max_tevs=128, tevs=0x880f88, pev=0x859b30) at util/probe-event.c:482
>   #7  convert_to_probe_trace_events (target=0x881b20
>       "/lib64/libpthread-2.14.90.so", max_tevs=128, tevs=0x880f88,
>       pev=0x859b30) at util/probe-event.c:2356
>   #8  add_perf_probe_events (pevs=<optimized out>, npevs=1, max_tevs=128,
>       target=0x881b20 "/lib64/libpthread-2.14.90.so", force_add=false) at
>       util/probe-event.c:2391
>   #9  0x000000000044014f in __cmd_probe (argc=<optimized out>,
>       argv=0x7fffffffe2f0, prefix=Unhandled dwarf expression opcode 0xfa) at
>       at builtin-probe.c:488
>   #10 0x0000000000440313 in cmd_probe (argc=5, argv=0x7fffffffe2f0,
>       prefix=<optimized out>) at builtin-probe.c:506
>   #11 0x000000000041d133 in run_builtin (p=0x805680, argc=5,
>       argv=0x7fffffffe2f0) at perf.c:341
>   #12 0x000000000041c8b2 in handle_internal_command (argv=<optimized out>,
>       argc=<optimized out>) at perf.c:400
>   #13 run_argv (argv=<optimized out>, argcp=<optimized out>) at perf.c:444
>   #14 main (argc=5, argv=0x7fffffffe2f0) at perf.c:559
> 
> And I found a related commit (5704c8c4fa71 "getcfi_scn_eh_frame: Don't
> crash and burn when .eh_frame bits aren't there.") in elfutils that
> can lead to a unexpected crash like this.  To safely use the function,
> it needs to check the .eh_frame section is a PROGBITS type.
> 

Looks good to me :)

> Reported-by: David Ahern <dsahern@gmail.com>
> Cc: Mark Wielaard <mjw@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thank you!

> ---
>  tools/perf/util/probe-finder.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index c7918f83b300..b5247d777f0e 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -989,8 +989,24 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
>  	int ret = 0;
>  
>  #if _ELFUTILS_PREREQ(0, 142)
> +	Elf *elf;
> +	GElf_Ehdr ehdr;
> +	GElf_Shdr shdr;
> +
>  	/* Get the call frame information from this dwarf */
> -	pf->cfi = dwarf_getcfi_elf(dwarf_getelf(dbg->dbg));
> +	elf = dwarf_getelf(dbg->dbg);
> +	if (elf == NULL)
> +		return -EINVAL;
> +
> +	if (gelf_getehdr(elf, &ehdr) == NULL)
> +		return -EINVAL;
> +
> +	if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
> +	    shdr.sh_type == SHT_PROGBITS) {
> +		pf->cfi = dwarf_getcfi_elf(elf);
> +	} else {
> +		pf->cfi = dwarf_getcfi(dbg->dbg);
> +	}
>  #endif
>  
>  	off = 0;
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



  parent reply	other threads:[~2014-12-30 22:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-30  4:39 perf-probe crash in dwarf_getcfi_elf David Ahern
2014-12-30  8:47 ` Namhyung Kim
2014-12-30  9:05   ` Namhyung Kim
2014-12-30 17:13     ` David Ahern
2014-12-30 22:13     ` Masami Hiramatsu [this message]
2014-12-31 19:42     ` Mark Wielaard
2015-01-03 15:07       ` Namhyung Kim
2015-01-08  9:52     ` [tip:perf/urgent] perf probe: Fix " tip-bot for Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54A3238B.90805@hitachi.com \
    --to=masami.hiramatsu.pt@hitachi.com \
    --cc=acme@kernel.org \
    --cc=dsahern@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjw@redhat.com \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.