* [PATCH V3 2/2] tools/perf: Use scnprintf in buffer offset calculations
2026-05-02 14:12 [PATCH V3 1/2] tools/perf: Fix the check for parameterized field in event term Athira Rajeev
@ 2026-05-02 14:12 ` Athira Rajeev
2026-05-04 1:49 ` [PATCH V3 1/2] tools/perf: Fix the check for parameterized field in event term Namhyung Kim
1 sibling, 0 replies; 4+ messages in thread
From: Athira Rajeev @ 2026-05-02 14:12 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
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 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 remaning 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:
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 | 46 ++++++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0b8d58543f17..4b9ade1a4cf9 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2129,15 +2129,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);
@@ -2201,6 +2205,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, len;
if (perf_pmu__is_tracepoint(pmu))
return tp_pmu__for_each_event(pmu, state, cb);
@@ -2234,17 +2239,36 @@ 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;
+ len = scnprintf(buf + buf_used, size_rem, "%G%s",
+ event->scale, event->unit);
+ /*
+ * Increment buf_used by 1 only if
+ * it fits remaining space
+ */
+ buf_used += min(len + 1, size_rem);
+ }
}
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;
+ len = scnprintf(buf + buf_used, size_rem, "%.*s/%s/",
+ (int)pmu_name_len, info.pmu_name, event->terms);
+ buf_used += min(len + 1, size_rem);
+ }
+
info.str = event->terms;
info.topic = event->topic;
info.deprecated = perf_pmu_alias__check_deprecated(pmu, event);
@@ -2254,7 +2278,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.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH V3 1/2] tools/perf: Fix the check for parameterized field in event term
2026-05-02 14:12 [PATCH V3 1/2] tools/perf: Fix the check for parameterized field in event term Athira Rajeev
2026-05-02 14:12 ` [PATCH V3 2/2] tools/perf: Use scnprintf in buffer offset calculations Athira Rajeev
@ 2026-05-04 1:49 ` Namhyung Kim
2026-05-04 4:17 ` Athira Rajeev
1 sibling, 1 reply; 4+ messages in thread
From: Namhyung Kim @ 2026-05-04 1:49 UTC (permalink / raw)
To: Athira Rajeev
Cc: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers,
linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1,
Tanushree.Shah, shivani
On Sat, May 02, 2026 at 07:42:57PM +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.
>
> 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.
Any real examples?
Thanks,
Namhyung
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> ---
> Changelog:
> 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 23337d2fa281..0b8d58543f17 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -2117,7 +2117,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.47.3
>
^ permalink raw reply [flat|nested] 4+ messages in thread