linux-kernel - Re: [PATCH] perf jevents: Fix event code for events referencing std arch events

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:   Tue, 13 Oct 2020 09:41:08 +0100
From:   John Garry <john.garry@...wei.com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <mark.rutland@....com>, <alexander.shishkin@...ux.intel.com>,
        <namhyung@...nel.org>, <kjain@...ux.ibm.com>, <irogers@...gle.com>,
        <yao.jin@...ux.intel.com>, <yeyunfeng@...wei.com>,
        <linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] perf jevents: Fix event code for events referencing std
 arch events

On 12/10/2020 12:24, Jiri Olsa wrote:
> On Mon, Oct 12, 2020 at 12:15:04PM +0100, John Garry wrote:
>> On 12/10/2020 11:54, Jiri Olsa wrote:
>>>> ff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
>>>> index 99df41a9543d..e47644cab3fa 100644
>>>> --- a/tools/perf/pmu-events/jevents.c
>>>> +++ b/tools/perf/pmu-events/jevents.c
>>>> @@ -505,20 +505,15 @@ static char *real_event(const char *name, char *event)
>>>>    }
>>>>    static int
>>>> -try_fixup(const char *fn, char *arch_std, unsigned long long eventcode,
>>>> -	  struct json_event *je)
>>>> +try_fixup(const char *fn, char *arch_std, struct json_event *je, char **event)
>>>>    {
>>>>    	/* try to find matching event from arch standard values */
>>>>    	struct event_struct *es;
>>>>    	list_for_each_entry(es, &arch_std_events, list) {
>>>>    		if (!strcmp(arch_std, es->name)) {
>>>> -			if (!eventcode && es->event) {
>>>> -				/* allow EventCode to be overridden */
>>>> -				free(je->event);
>>>> -				je->event = NULL;
>>>> -			}
>>>>    			FOR_ALL_EVENT_STRUCT_FIELDS(TRY_FIXUP_FIELD);
>>>> +			*event = je->event;
>>> I'm bit rusty on this code, but isn't je->event NULL at this point?
>>
>> je->event should be now assigned from es->event because of
>> FOR_ALL_EVENT_STRUCT_FIELDS(TRY_FIXUP_FIELD):
>>
>> #define TRY_FIXUP_FIELD(field) do { if (es->field && !*field) {\
>> 	*field = strdup(es->field);				\
>> 	if (!*field)						\
>> 		return -ENOMEM;					\
>> } } while (0)
>>
>> And es->event should be set.
> 
> right, thanks
> 
> Acked-by: Jiri Olsa <jolsa@...hat.com>
> 
> jirka
> 
> .
> 

"PMU events" testcase was still passing as it does not cover this 
scenario, so I'll look to expand the test to catch such problems.

Thanks,
John