* [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term
@ 2026-06-09 13:43 Athira Rajeev
2026-06-09 13:43 ` [PATCH V5 2/2] tools/perf: Use scnprintf in buffer offset calculations Athira Rajeev
2026-06-10 20:07 ` [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Arnaldo Carvalho de Melo
0 siblings, 2 replies; 4+ messages in thread
From: Athira Rajeev @ 2026-06-09 13:43 UTC (permalink / raw)
To: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers,
namhyung
Cc: linux-perf-users, linuxppc-dev, atrajeev, hbathini, Tejas.Manhas1,
Tanushree.Shah, shivani, venkat88
The format_alias() function in util/pmu.c has a check to
detect whether the event has parameterized field ( =? ).
The string alias->terms contains the event and if the event
has user configurable parameter, there will be presence of
sub string "=?" in the alias->terms.
Snippet of code:
/* Paramemterized events have the parameters shown. */
if (strstr(alias->terms, "=?")) {
/* No parameters. */
snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
if "strstr" contains the substring, it returns a pointer
and hence enters the above check which is not the expected
check. And hence "perf list" doesn't have the parameterized
fields in the result.
Fix this check to use:
if (!strstr(alias->terms, "=?")) {
With this change, perf list shows the events correctly with
the strings showing parameters.
Before the fix:
# ./perf list|grep -w PM_PAU_CYC
hv_24x7/PM_PAU_CYC/ [Kernel PMU event]
With this fix:
# ./perf list|grep -w PM_PAU_CYC
hv_24x7/PM_PAU_CYC,chip=?/ [Kernel PMU event]
Reviewed-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changelog:
v4 -> v5:
Added Reviewed-by from Ian and Namhyung.
Added Tested-by from Venkat
v3 -> v4:
Updated commit message to show real example
addressing review comment from Namhyung.
v2 -> v3:
Split the strstr correction in a single patch
tools/perf/util/pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9994709ef12b..e765a7ffb0d6 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2134,7 +2134,7 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
skip_duplicate_pmus);
/* Paramemterized events have the parameters shown. */
- if (strstr(alias->terms, "=?")) {
+ if (!strstr(alias->terms, "=?")) {
/* No parameters. */
snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
return buf;
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH V5 2/2] tools/perf: Use scnprintf in buffer offset calculations 2026-06-09 13:43 [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Athira Rajeev @ 2026-06-09 13:43 ` Athira Rajeev 2026-06-10 20:07 ` [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Arnaldo Carvalho de Melo 1 sibling, 0 replies; 4+ messages in thread From: Athira Rajeev @ 2026-06-09 13:43 UTC (permalink / raw) To: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers, namhyung Cc: linux-perf-users, linuxppc-dev, atrajeev, hbathini, Tejas.Manhas1, Tanushree.Shah, shivani, venkat88 Replace snprintf with scnprintf in buffer offset calculations to ensure the 'used' count will not exceed the "len". The current logic in perf_pmu__for_each_event uses an unconditional + 1 increment to buf_used to account for null terminators. This can cause a stack buffer overflow in the subsequent scnprintf call. When the local stack buffer buf (1024 bytes) is full, buf_used can reach 1025. This causes the subsequent remaining space calculation sizeof(buf) - buf_used to underflow. Use sub_non_neg() to see if space actually existed, and only increment the offset if remaining space is present. Changes includes: - Use sub_non_neg to check if space exists - Replacing snprintf with scnprintf to ensure the return value reflects the actual bytes written into the buffer. - Only increment buf_used by 1 if space exists - If a parameterized event uses a built-in perf keyword for its parameter name (eg, config=?), the lexer parses it as a predefined term token, which sets term->config to NULL. Add check to use parse_events__term_type_str() if term->config is NULL. Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com> --- Changelog: v4 -> v5: Addressed review comment from Namhyung in buf_used variable to use return from scnprintf since cannot return a number greater than or equal to argument v2 -> v3: - Split the scnprintf related changes in separate patch - Handle the overflow issues and unconditional increment wrapped around sub_non_neg addressing review comment from Sashiko tools/perf/util/pmu.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index e765a7ffb0d6..1539960ba23b 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -2146,15 +2146,19 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu, pr_err("Failure to parse '%s' terms '%s': %d\n", alias->name, alias->terms, ret); parse_events_terms__exit(&terms); - snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name); + scnprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name); return buf; } - used = snprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name); + used = scnprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name); list_for_each_entry(term, &terms.terms, list) { + const char *name = term->config; + + if (!name) + name = parse_events__term_type_str(term->type_term); if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) - used += snprintf(buf + used, sub_non_neg(len, used), - ",%s=%s", term->config, + used += scnprintf(buf + used, sub_non_neg(len, used), + ",%s=%s", name, term->val.str); } parse_events_terms__exit(&terms); @@ -2218,6 +2222,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus, int ret = 0; struct hashmap_entry *entry; size_t bkt; + size_t size_rem; if (perf_pmu__is_tracepoint(pmu)) return tp_pmu__for_each_event(pmu, state, cb); @@ -2251,17 +2256,30 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus, } buf_used = strlen(buf) + 1; } + info.scale_unit = NULL; if (strlen(event->unit) || event->scale != 1.0) { - info.scale_unit = buf + buf_used; - buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used, - "%G%s", event->scale, event->unit) + 1; + /* Check the remaining space */ + size_rem = sub_non_neg(sizeof(buf), buf_used); + + if (size_rem > 0) { + info.scale_unit = buf + buf_used; + buf_used += scnprintf(buf + buf_used, size_rem, "%G%s", + event->scale, event->unit) + 1; + } } info.desc = event->desc; info.long_desc = event->long_desc; - info.encoding_desc = buf + buf_used; - buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used, - "%.*s/%s/", (int)pmu_name_len, info.pmu_name, event->terms) + 1; + info.encoding_desc = NULL; + + /* Check the remaining space */ + size_rem = sub_non_neg(sizeof(buf), buf_used); + if (size_rem > 0) { + info.encoding_desc = buf + buf_used; + buf_used += scnprintf(buf + buf_used, size_rem, "%.*s/%s/", + (int)pmu_name_len, info.pmu_name, event->terms) + 1; + } + info.str = event->terms; info.topic = event->topic; info.deprecated = perf_pmu_alias__check_deprecated(pmu, event); @@ -2271,7 +2289,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus, } if (pmu->selectable) { info.name = buf; - snprintf(buf, sizeof(buf), "%s//", pmu->name); + scnprintf(buf, sizeof(buf), "%s//", pmu->name); info.alias = NULL; info.scale_unit = NULL; info.desc = NULL; -- 2.52.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term 2026-06-09 13:43 [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Athira Rajeev 2026-06-09 13:43 ` [PATCH V5 2/2] tools/perf: Use scnprintf in buffer offset calculations Athira Rajeev @ 2026-06-10 20:07 ` Arnaldo Carvalho de Melo 2026-06-11 12:50 ` Athira Rajeev 1 sibling, 1 reply; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-06-10 20:07 UTC (permalink / raw) To: Athira Rajeev Cc: jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers, namhyung, linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1, Tanushree.Shah, shivani, venkat88 On Tue, Jun 09, 2026 at 07:13:31PM +0530, Athira Rajeev wrote: > The format_alias() function in util/pmu.c has a check to > detect whether the event has parameterized field ( =? ). > The string alias->terms contains the event and if the event > has user configurable parameter, there will be presence of > sub string "=?" in the alias->terms. Thanks, applied to perf-tools-next, for v7.2. - Arnaldo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term 2026-06-10 20:07 ` [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Arnaldo Carvalho de Melo @ 2026-06-11 12:50 ` Athira Rajeev 0 siblings, 0 replies; 4+ messages in thread From: Athira Rajeev @ 2026-06-11 12:50 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers, namhyung, linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1, Tanushree.Shah, shivani, venkat88 > On 11 Jun 2026, at 1:37 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > On Tue, Jun 09, 2026 at 07:13:31PM +0530, Athira Rajeev wrote: >> The format_alias() function in util/pmu.c has a check to >> detect whether the event has parameterized field ( =? ). >> The string alias->terms contains the event and if the event >> has user configurable parameter, there will be presence of >> sub string "=?" in the alias->terms. > > Thanks, applied to perf-tools-next, for v7.2. > > - Arnaldo Thanks Arnaldo Athira ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-11 12:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-09 13:43 [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Athira Rajeev 2026-06-09 13:43 ` [PATCH V5 2/2] tools/perf: Use scnprintf in buffer offset calculations Athira Rajeev 2026-06-10 20:07 ` [PATCH V5 1/2] tools/perf: Fix the check for parameterized field in event term Arnaldo Carvalho de Melo 2026-06-11 12:50 ` Athira Rajeev
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.