Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757806Ab2BIPHV (ORCPT ); Thu, 9 Feb 2012 10:07:21 -0500 Received: from mail-wi0-f174.google.com ([209.85.212.174]:57507 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751215Ab2BIPHU convert rfc822-to-8bit (ORCPT ); Thu, 9 Feb 2012 10:07:20 -0500 MIME-Version: 1.0 In-Reply-To: References: <20120209103016.GA1999@quad> <20120209144844.GA4998@infradead.org> From: Sorin Dumitru Date: Thu, 9 Feb 2012 17:06:57 +0200 Message-ID: Subject: Re: [PATCH] perf: add sanity check on addr in symbol__inc_addr_samples() To: Stephane Eranian Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@elte.hu Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2719 Lines: 68 On Thu, Feb 9, 2012 at 4:53 PM, Stephane Eranian wrote: > On Thu, Feb 9, 2012 at 3:48 PM, Arnaldo Carvalho de Melo > wrote: >> Em Thu, Feb 09, 2012 at 11:30:16AM +0100, Stephane Eranian escreveu: >>> >>> Check the value of addr against the bounds of the symbol. >>> This is needed given we compute an offset: >>> ? ? ? offset = addr - sym->start >>> >>> And we don't want the offset to become negative. >> >> I'll try and add a debug option to show the backtrace and values of >> addr, sym, etc, so that we can fix the real problem. >> >> I.e. this function shouldn't be receiving any such invalid addresses, as >> the symbol lookup was done, the symbol was found to be this one, then >> why it would be out of bounds at this point?! >> > I tend to agree with you on this. But then I don't see why the first test > was there. > >> - Arnaldo >> >>> Signed-off-by: Stephane Eranian >>> >>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>> index 011ed26..8248d80 100644 >>> --- a/tools/perf/util/annotate.c >>> +++ b/tools/perf/util/annotate.c >>> @@ -64,7 +64,7 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map, >>> >>> ? ? ? pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map->unmap_ip(map, addr)); >>> >>> - ? ? if (addr >= sym->end) >>> + ? ? if (addr >= sym->end || addr < sym->start) >>> ? ? ? ? ? ? ? return 0; >>> >>> ? ? ? offset = addr - sym->start; > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at ?http://vger.kernel.org/majordomo-info.html > Please read the FAQ at ?http://www.tux.org/lkml/ I reported the same problem a couple of weeks ago. From what i can tell the problem is in perf_event__process_sample. When calling perf_event__process_sample, we set al->sym based on al->address. The symbol in the hist_entry is set to the one from al but in the call to perf_top__record_precise_ip we pass in the address from the event struct which is sometimes different than the one in the al structure. When this situation occurs, when calculating the offset in symbol__inc_addr_samples, because addr is not in the symbol [start,end] range, we get a very big value which causes the segfault when we use it to index something. I've sent a patch that works for me, but i don't know if it's the right solution at [1]. [1] https://lkml.org/lkml/2012/1/29/59 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/