linux-kernel - Re: [PATCH v2] perf probe: Clear all structure fields in clear_perf_{probe, trace}

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] [day] [month] [year] [list]

Message-Id: <20241125151437.9cb726157de25cfba0a44665@kernel.org>
Date: Mon, 25 Nov 2024 15:14:37 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Li Huafei <lihuafei1@...wei.com>
Cc: <acme@...nel.org>, <peterz@...radead.org>, <mingo@...hat.com>,
 <namhyung@...nel.org>, <mark.rutland@....com>,
 <alexander.shishkin@...ux.intel.com>, <jolsa@...nel.org>,
 <irogers@...gle.com>, <adrian.hunter@...el.com>,
 <kan.liang@...ux.intel.com>, <dima@...retsauce.net>,
 <aleksander.lobakin@...el.com>, <linux-perf-users@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] perf probe: Clear all structure fields in
 clear_perf_{probe, trace}_event()

On Thu, 14 Nov 2024 18:54:51 +0800
Li Huafei <lihuafei1@...wei.com> wrote:

> I added two probe events:
> 
>   # perf probe -f -a schedule+8
>   Added new event:
>     probe:schedule       (on schedule+8)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:schedule -aR sleep 1
> 
>   # perf probe -f -a schedule+20
>   Added new event:
>     probe:schedule_1     (on schedule+20)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:schedule_1 -aR sleep 1
> 
> However, 'perf probe -l' shows the same offset:
> 
>   # perf probe -l
>     probe:schedule       (on schedule+8@...nel/sched/core.c)
>     probe:schedule_1     (on schedule+8@...nel/sched/core.c)
> 
> __show_perf_probe_events() does not clean up the 'pev' content when
> parsing the rawlist. If the 'pev->offset' is not set while processing
> the next probe event string, the offset value of the previous event will
> be used.  After adding debug information, it was found that indeed there
> was line number information when processing 'probe:schedule_1', so the
> offset was not set and used the offset from 'probe:schedule'.
> 
> To fix this, clear all the fields of the structures in
> clear_perf_{probe, trace}_event(). not just the allocated fields.
> 

Looks good to me.

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

Thanks!

> Fixes: d8f9da240495 ("perf tools: Use zfree() where applicable")
> Signed-off-by: Li Huafei <lihuafei1@...wei.com>
> ---
> Changes in v2:
>  - Do the cleanup in clear_perf_{probe, trace}_event().
>  - Refine the subject and the commit log.
> 
> v1: https://lore.kernel.org/lkml/20241108181909.3515716-1-lihuafei1@huawei.com/
> ---
>  tools/perf/util/probe-event.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a17c9b8a7a79..47903bb56fc6 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2376,8 +2376,8 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
>  			field = next;
>  		}
>  	}
> -	pev->nargs = 0;
>  	zfree(&pev->args);
> +	memset(pev, 0, sizeof(*pev));
>  }
>  
>  #define strdup_or_goto(str, label)	\
> @@ -2475,7 +2475,7 @@ void clear_probe_trace_event(struct probe_trace_event *tev)
>  		}
>  	}
>  	zfree(&tev->args);
> -	tev->nargs = 0;
> +	memset(tev, 0, sizeof(*tev));
>  }
>  
>  struct kprobe_blacklist_node {
> -- 
> 2.25.1
> 


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