Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
@ 2026-02-17 13:14 Thomas Richter
  2026-02-17 14:09 ` James Clark
  2026-02-24 17:57 ` Namhyung Kim
  0 siblings, 2 replies; 7+ messages in thread
From: Thomas Richter @ 2026-02-17 13:14 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	james.clark, irogers
  Cc: agordeev, gor, sumanthk, hca, japo, Thomas Richter

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;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
  2026-02-17 13:14 [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member Thomas Richter
@ 2026-02-17 14:09 ` James Clark
  2026-02-20  9:31   ` PING " Thomas Richter
  2026-02-24 16:47   ` Ian Rogers
  2026-02-24 17:57 ` Namhyung Kim
  1 sibling, 2 replies; 7+ messages in thread
From: James Clark @ 2026-02-17 14:09 UTC (permalink / raw)
  To: Thomas Richter
  Cc: agordeev, gor, sumanthk, hca, japo, linux-kernel, linux-s390,
	linux-perf-users, acme, namhyung, irogers



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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* PING [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
  2026-02-17 14:09 ` James Clark
@ 2026-02-20  9:31   ` Thomas Richter
  2026-02-20 23:53     ` Namhyung Kim
  2026-02-24 16:47   ` Ian Rogers
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Richter @ 2026-02-20  9:31 UTC (permalink / raw)
  To: James Clark, Namhyung Kim, Arnaldo Carvalho de Melo, Ian Rogers
  Cc: linux-perf-users

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: PING [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
  2026-02-20  9:31   ` PING " Thomas Richter
@ 2026-02-20 23:53     ` Namhyung Kim
  2026-02-21  8:06       ` Arnaldo Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2026-02-20 23:53 UTC (permalink / raw)
  To: Thomas Richter
  Cc: James Clark, Arnaldo Carvalho de Melo, Ian Rogers,
	linux-perf-users

Hello,

On Fri, Feb 20, 2026 at 10:31:17AM +0100, Thomas Richter wrote:
> 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...

Sorry for the delay, I'll queue it up after the merge window.

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: PING [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
  2026-02-20 23:53     ` Namhyung Kim
@ 2026-02-21  8:06       ` Arnaldo Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Melo @ 2026-02-21  8:06 UTC (permalink / raw)
  To: Namhyung Kim, Thomas Richter
  Cc: James Clark, Arnaldo Carvalho de Melo, Ian Rogers,
	linux-perf-users



On February 20, 2026 8:53:07 PM GMT-03:00, Namhyung Kim <namhyung@kernel.org> wrote:
>Hello,
>
>On Fri, Feb 20, 2026 at 10:31:17AM +0100, Thomas Richter wrote:
>> 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...
>
>Sorry for the delay, I'll queue it up after the merge window.
>

Thanks for the ping, I'll consider it later today, when I plan to send the pull request for this merge window. 

- Arnaldo

>Thanks,
>Namhyung
>

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
  2026-02-17 14:09 ` James Clark
  2026-02-20  9:31   ` PING " Thomas Richter
@ 2026-02-24 16:47   ` Ian Rogers
  1 sibling, 0 replies; 7+ messages in thread
From: Ian Rogers @ 2026-02-24 16:47 UTC (permalink / raw)
  To: James Clark
  Cc: Thomas Richter, agordeev, gor, sumanthk, hca, japo, linux-kernel,
	linux-s390, linux-perf-users, acme, namhyung

On Tue, Feb 17, 2026 at 6:09 AM James Clark <james.clark@linaro.org> 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>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member
  2026-02-17 13:14 [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member Thomas Richter
  2026-02-17 14:09 ` James Clark
@ 2026-02-24 17:57 ` Namhyung Kim
  1 sibling, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-02-24 17:57 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linux-perf-users, acme, james.clark,
	irogers, Thomas Richter
  Cc: agordeev, gor, sumanthk, hca, japo

On Tue, 17 Feb 2026 14:14:56 +0100, 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.
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-02-24 17:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-17 13:14 [PATCH v4 linux-next] perf parse-events: Fix big-endian 'overwrite' by writing correct union member Thomas Richter
2026-02-17 14:09 ` James Clark
2026-02-20  9:31   ` PING " Thomas Richter
2026-02-20 23:53     ` Namhyung Kim
2026-02-21  8:06       ` Arnaldo Melo
2026-02-24 16:47   ` Ian Rogers
2026-02-24 17:57 ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox