linux-kernel - Re: [PATCH 2/9] perf annotate-data: Fix off-by-one in location range check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240818222221.fd9ced7456084b490c52b2da@kernel.org>
Date: Sun, 18 Aug 2024 22:22:21 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers
 <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>, Jiri Olsa
 <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, Peter Zijlstra
 <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, LKML
 <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org, Athira
 Rajeev <atrajeev@...ux.vnet.ibm.com>, Masami Hiramatsu
 <mhiramat@...nel.org>
Subject: Re: [PATCH 2/9] perf annotate-data: Fix off-by-one in location
 range check

On Fri, 16 Aug 2024 16:58:32 -0700
Namhyung Kim <namhyung@...nel.org> wrote:

> The location list will have entries with half-open addressing like
> [start, end) which means it doesn't include the end address.  So it
> should skip entries at the end address and match to the next entry.
> 
> An example location list looks like this:
> 
>     00237876 ffffffff8110d32b (base address)
>     0023787f v000000000000000 v000000000000002 views at 00237868 for:
>              ffffffff8110d32b ffffffff8110d4eb (DW_OP_reg3 (rbx))     <<<--- 1
>     00237885 v000000000000002 v000000000000000 views at 0023786a for:
>              ffffffff8110d4eb ffffffff8110d50b (DW_OP_reg14 (r14))    <<<--- 2
>     0023788c v000000000000000 v000000000000001 views at 0023786c for:
>              ffffffff8110d50b ffffffff8110d7c4 (DW_OP_reg3 (rbx))
>     00237893 v000000000000000 v000000000000000 views at 0023786e for:
>              ffffffff8110d806 ffffffff8110d854 (DW_OP_reg3 (rbx))
>     0023789a v000000000000000 v000000000000000 views at 00237870 for:
>              ffffffff8110d876 ffffffff8110d88e (DW_OP_reg3 (rbx))
> 
> The first entry at 0023787f has [8110d32b, 8110d4eb) (omitting the
> ffffffff at the beginning), and the second one has [8110d4eb, 8110d50b).
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>

Thanks!

> Fixes: 2bc3cf575a16 ("perf annotate-data: Improve debug message with location info")
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/annotate-data.c | 2 +-
>  tools/perf/util/dwarf-aux.c     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index ff85d190e3ac..fd8d3cdead5a 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -95,7 +95,7 @@ static void pr_debug_location(Dwarf_Die *die, u64 pc, int reg)
>  		return;
>  
>  	while ((off = dwarf_getlocations(&attr, off, &base, &start, &end, &ops, &nops)) > 0) {
> -		if (reg != DWARF_REG_PC && end < pc)
> +		if (reg != DWARF_REG_PC && end <= pc)
>  			continue;
>  		if (reg != DWARF_REG_PC && start > pc)
>  			break;
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index beb632153a74..0151a8d14350 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1444,7 +1444,7 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
>  
>  	while ((off = dwarf_getlocations(&attr, off, &base, &start, &end, &ops, &nops)) > 0) {
>  		/* Assuming the location list is sorted by address */
> -		if (end < data->pc)
> +		if (end <= data->pc)
>  			continue;
>  		if (start > data->pc)
>  			break;
> -- 
> 2.46.0.184.g6999bdac58-goog
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>