linux-kernel - Re: [PATCH 4/4] perf mem: Fix a segfault with NULL event->name

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]

Date: Fri, 21 Jun 2024 16:11:43 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Namhyung Kim <namhyung@...nel.org>,
 Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers <irogers@...gle.com>
Cc: 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,
 Guilherme Amadio <amadio@...n.ch>
Subject: Re: [PATCH 4/4] perf mem: Fix a segfault with NULL event->name



On 2024-06-21 1:05 p.m., Namhyung Kim wrote:
> Guilherme reported a crash in perf mem record.  It's because the
> perf_mem_event->name was NULL on his machine.  It should just return
> a NULL string when it has no format string in the name.
> 
> But I'm not sure why it returns TRUE if it doesn't have event_name in
> perf_pmu__mem_events_supported().

AMD doesn't have the event_name.

struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
	E(NULL,		NULL,		NULL,	false,	0),
	E(NULL,		NULL,		NULL,	false,	0),
	E("mem-ldst",	"%s//",		NULL,	false,	0),
};

It looks like as long as it's a "ibs_op" PMU, the mem event is always
available. The "ibs_op" check has been moved into perf_pmu__arch_init().
So the event_name is empty.

The "e->tag" in the perf_pmu__mem_events_init() can help to skip the
case  E(NULL,		NULL,		NULL,	false,	0).
So the perf_pmu__mem_events_supported() returns TRUE for the !event_name.

Thanks,
Kan
> 
> The backtrace at the crash is below:
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   __strchrnul_avx2 () at ../sysdeps/x86_64/multiarch/strchr-avx2.S:67
>   67              vmovdqu (%rdi), %ymm2
>   (gdb) bt
>   #0  __strchrnul_avx2 () at ../sysdeps/x86_64/multiarch/strchr-avx2.S:67
>   #1  0x00007ffff6c982de in __find_specmb (format=0x0) at printf-parse.h:82
>   #2  __printf_buffer (buf=buf@...ry=0x7fffffffc760, format=format@...ry=0x0, ap=ap@...ry=0x7fffffffc880,
>       mode_flags=mode_flags@...ry=0) at vfprintf-internal.c:649
>   #3  0x00007ffff6cb7840 in __vsnprintf_internal (string=<optimized out>, maxlen=<optimized out>, format=0x0,
>       args=0x7fffffffc880, mode_flags=mode_flags@...ry=0) at vsnprintf.c:96
>   #4  0x00007ffff6cb787f in ___vsnprintf (string=<optimized out>, maxlen=<optimized out>, format=<optimized out>,
>       args=<optimized out>) at vsnprintf.c:103
>   #5  0x00005555557b9391 in scnprintf (buf=0x555555fe9320 <mem_loads_name> "", size=100, fmt=0x0)
>       at ../lib/vsprintf.c:21
>   #6  0x00005555557b74c3 in perf_pmu__mem_events_name (i=0, pmu=0x555556832180) at util/mem-events.c:106
>   #7  0x00005555557b7ab9 in perf_mem_events__record_args (rec_argv=0x55555684c000, argv_nr=0x7fffffffca20)
>       at util/mem-events.c:252
>   #8  0x00005555555e370d in __cmd_record (argc=3, argv=0x7fffffffd760, mem=0x7fffffffcd80) at builtin-mem.c:156
>   #9  0x00005555555e49c4 in cmd_mem (argc=4, argv=0x7fffffffd760) at builtin-mem.c:514
>   #10 0x000055555569716c in run_builtin (p=0x555555fcde80 <commands+672>, argc=8, argv=0x7fffffffd760) at perf.c:349
>   #11 0x0000555555697402 in handle_internal_command (argc=8, argv=0x7fffffffd760) at perf.c:402
>   #12 0x0000555555697560 in run_argv (argcp=0x7fffffffd59c, argv=0x7fffffffd590) at perf.c:446
>   #13 0x00005555556978a6 in main (argc=8, argv=0x7fffffffd760) at perf.c:562
> 
> Reported-by: Guilherme Amadio <amadio@...n.ch>
> Closes: https://lore.kernel.org/linux-perf-users/Zlns_o_IE5L28168@cern.ch
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/mem-events.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 6dda47bb774f..429079329e48 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -86,7 +86,7 @@ static const char *perf_pmu__mem_events_name(int i, struct perf_pmu *pmu)
>  		return NULL;
>  
>  	e = &pmu->mem_events[i];
> -	if (!e)
> +	if (!e || !e->name)
>  		return NULL;
>  
>  	if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE) {