linux-kernel - Re: [PATCH 1/9] perf dwarf-aux: Check allowed location expressions when collecting variables

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: <20240818012411.a00eb93e5b8f9cf45e2f513b@kernel.org>
Date: Sun, 18 Aug 2024 01:24:11 +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 1/9] perf dwarf-aux: Check allowed location expressions
 when collecting variables

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

> It missed to call check_allowed_ops() in __die_collect_vars_cb() so it
> can take variables with complex location expression incorrectly.
> 
> For example, I found some variable has this expression.
> 
>     015d8df8 ffffffff81aacfb3 (base address)
>     015d8e01 v000000000000004 v000000000000000 views at 015d8df2 for:
>              ffffffff81aacfb3 ffffffff81aacfd2 (DW_OP_fbreg: -176; DW_OP_deref;
> 						DW_OP_plus_uconst: 332; DW_OP_deref_size: 4;
> 						DW_OP_lit1; DW_OP_shra; DW_OP_const1u: 64;
> 						DW_OP_minus; DW_OP_stack_value)
>     015d8e14 v000000000000000 v000000000000000 views at 015d8df4 for:
>              ffffffff81aacfd2 ffffffff81aacfd7 (DW_OP_reg3 (rbx))
>     015d8e19 v000000000000000 v000000000000000 views at 015d8df6 for:
>              ffffffff81aacfd7 ffffffff81aad020 (DW_OP_fbreg: -176; DW_OP_deref;
> 						DW_OP_plus_uconst: 332; DW_OP_deref_size: 4;
> 						DW_OP_lit1; DW_OP_shra; DW_OP_const1u: 64;
> 						DW_OP_minus; DW_OP_stack_value)
>     015d8e2c <End of list>
> 
> It looks like '((int *)(-176(%rbp) + 332) >> 1) - 64' but the current
> code thought it's just -176(%rbp) and processed the variable incorrectly.
> It should reject such a complex expression if check_allowed_ops()
> doesn't like it. :)

Looks good to me. Hmm, I should reconsider to support this complex variable
on ftrace probe events...

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

Thanks,

> 
> Fixes: 932dcc2c39ae ("perf dwarf-aux: Add die_collect_vars()")
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/dwarf-aux.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 5e080d7e22c2..beb632153a74 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1598,6 +1598,9 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
>  	if (dwarf_getlocations(&attr, 0, &base, &start, &end, &ops, &nops) <= 0)
>  		return DIE_FIND_CB_SIBLING;
>  
> +	if (!check_allowed_ops(ops, nops))
> +		return DIE_FIND_CB_SIBLING;
> +
>  	if (die_get_real_type(die_mem, &type_die) == NULL)
>  		return DIE_FIND_CB_SIBLING;
>  
> -- 
> 2.46.0.184.g6999bdac58-goog
> 
> 


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