PING [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member

[Thomas Richter <tmricht@linux.ibm.com>]

On 2/17/26 15:09, James Clark wrote:
> 
> 
> On 17/02/2026 1:14 pm, Thomas Richter wrote:
>> v1 -> v2: Add comments from James Clark and Ian Rogers
>> v2 -> v3: Improve switch statement in add_config_term()
>> v3 -> v4: Fix clang warnings
>>
>> The "Read backward ring buffer" test crashes on big-endian (e.g. s390x)
>> due to a NULL dereference when the backward mmap path isn't enabled.
>>
>> Reproducer:
>>    # ./perf test -F 'Read backward ring buffer'
>>    Segmentation fault (core dumped)
>>    # uname -m
>>    s390x
>>    #
>>
>> Root cause:
>> get_config_terms() stores into evsel_config_term::val.val (u64) while later
>> code reads boolean fields such as evsel_config_term::val.overwrite.
>> On big-endian the 1-byte boolean is left-aligned, so writing
>> evsel_config_term::val.val = 1 is read back as
>> evsel_config_term::val.overwrite = 0,
>> leaving backward mmap disabled and a NULL map being used.
>>
>> Store values in the union member that matches the term type, e.g.:
>>    /* for OVERWRITE */
>>    new_term->val.overwrite = 1;  /* not new_term->val.val = 1 */
>> to fix this. Improve add_config_term() and add two more parameters for
>> string and value. Function add_config_term() now creates a complete node
>> element of type evsel_config_term and handles all evsel_config_term::val
>> union members.
>>
>> Impact:
>> Enables backward mmap on big-endian and prevents the crash.
>> No change on little-endian.
>>
>> Output after:
>>   # ./perf test -Fv 44
>>   --- start ---
>>   Using CPUID IBM,9175,705,ME1,3.8,002f
>>   mmap size 1052672B
>>   mmap size 8192B
>>   ---- end ----
>>   44: Read backward ring buffer                         : Ok
>>   #
>>
>> Fixes: 159ca97cd97c ("perf parse-events: Refactor get_config_terms() to remove macros")
>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>> Reviewed-by: Jan Polensky <japo@linux.ibm.com>
>> Cc: James Clark <james.clark@linaro.org>
>> Cc: Ian Rogers <irogers@google.com>
>> ---
>>   tools/perf/util/parse-events.c | 82 +++++++++++++++++++++++++++-------
>>   1 file changed, 65 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index b9efb296bba5..7b4629625b1e 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1117,7 +1117,7 @@ static int config_attr(struct perf_event_attr *attr,
>>     static struct evsel_config_term *add_config_term(enum evsel_term_type type,
>>                            struct list_head *head_terms,
>> -                         bool weak)
>> +                         bool weak, char *str, u64 val)
>>   {
>>       struct evsel_config_term *t;
>>   @@ -1128,8 +1128,62 @@ static struct evsel_config_term *add_config_term(enum evsel_term_type type,
>>       INIT_LIST_HEAD(&t->list);
>>       t->type = type;
>>       t->weak    = weak;
>> +
>> +    switch (type) {
>> +    case EVSEL__CONFIG_TERM_PERIOD:
>> +    case EVSEL__CONFIG_TERM_FREQ:
>> +    case EVSEL__CONFIG_TERM_STACK_USER:
>> +    case EVSEL__CONFIG_TERM_USR_CHG_CONFIG:
>> +    case EVSEL__CONFIG_TERM_USR_CHG_CONFIG1:
>> +    case EVSEL__CONFIG_TERM_USR_CHG_CONFIG2:
>> +    case EVSEL__CONFIG_TERM_USR_CHG_CONFIG3:
>> +    case EVSEL__CONFIG_TERM_USR_CHG_CONFIG4:
>> +        t->val.val = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_TIME:
>> +        t->val.time = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_INHERIT:
>> +        t->val.inherit = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_OVERWRITE:
>> +        t->val.overwrite = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_MAX_STACK:
>> +        t->val.max_stack = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_MAX_EVENTS:
>> +        t->val.max_events = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_PERCORE:
>> +        t->val.percore = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_AUX_OUTPUT:
>> +        t->val.aux_output = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE:
>> +        t->val.aux_sample_size = val;
>> +        break;
>> +    case EVSEL__CONFIG_TERM_CALLGRAPH:
>> +    case EVSEL__CONFIG_TERM_BRANCH:
>> +    case EVSEL__CONFIG_TERM_DRV_CFG:
>> +    case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
>> +    case EVSEL__CONFIG_TERM_AUX_ACTION:
>> +        if (str) {
>> +            t->val.str = strdup(str);
>> +            if (!t->val.str) {
>> +                zfree(&t);
>> +                return NULL;
>> +            }
>> +            t->free_str = true;
>> +        }
>> +        break;
>> +    default:
>> +        t->val.val = val;
>> +        break;
>> +    }
>> +
>>       list_add_tail(&t->list, head_terms);
>> -
>>       return t;
>>   }
>>   @@ -1142,7 +1196,7 @@ static int get_config_terms(const struct parse_events_terms *head_config,
>>           struct evsel_config_term *new_term;
>>           enum evsel_term_type new_type;
>>           bool str_type = false;
>> -        u64 val;
>> +        u64 val = 0;
>>             switch (term->type_term) {
>>           case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
>> @@ -1234,20 +1288,15 @@ static int get_config_terms(const struct parse_events_terms *head_config,
>>               continue;
>>           }
>>   -        new_term = add_config_term(new_type, head_terms, term->weak);
>> +        /*
>> +         * Note: Members evsel_config_term::val and
>> +         * parse_events_term::val are unions and endianness needs
>> +         * to be taken into account when changing such union members.
>> +         */
>> +        new_term = add_config_term(new_type, head_terms, term->weak,
>> +                       str_type ? term->val.str : NULL, val);
>>           if (!new_term)
>>               return -ENOMEM;
>> -
>> -        if (str_type) {
>> -            new_term->val.str = strdup(term->val.str);
>> -            if (!new_term->val.str) {
>> -                zfree(&new_term);
>> -                return -ENOMEM;
>> -            }
>> -            new_term->free_str = true;
>> -        } else {
>> -            new_term->val.val = val;
>> -        }
>>       }
>>       return 0;
>>   }
>> @@ -1277,10 +1326,9 @@ static int add_cfg_chg(const struct perf_pmu *pmu,
>>       if (bits) {
>>           struct evsel_config_term *new_term;
>>   -        new_term = add_config_term(new_term_type, head_terms, false);
>> +        new_term = add_config_term(new_term_type, head_terms, false, NULL, bits);
>>           if (!new_term)
>>               return -ENOMEM;
>> -        new_term->val.cfg_chg = bits;
>>       }
>>         return 0;
> 
> Reviewed-by: James Clark <james.clark@linaro.org>
> 

Friendly ping, this still fails in linux-next...

Thanks a lot

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294