* [PATCH v1] perf header: Ensure read strings are '\0' terminated
@ 2026-04-14 20:57 Ian Rogers
2026-04-14 21:15 ` David Laight
2026-04-14 21:21 ` David Laight
0 siblings, 2 replies; 3+ messages in thread
From: Ian Rogers @ 2026-04-14 20:57 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, Swapnil Sapkal,
Anubhav Shelat, linux-perf-users, linux-kernel
Sashiko reviews were complaining do_read_string didn't necessarily
ensure strings were correctly terminated. Add checking for this and if
a string isn't correctly terminated return NULL.
Signed-off-by: Ian Rogers <irogers@google.com>
---
| 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index f30e48eb3fc3..fa4f6d773874 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -269,6 +269,9 @@ static char *do_read_string(struct feat_fd *ff)
if (do_read_u32(ff, &len))
return NULL;
+ if (len == 0)
+ return NULL;
+
buf = malloc(len);
if (!buf)
return NULL;
@@ -279,7 +282,10 @@ static char *do_read_string(struct feat_fd *ff)
* thus the actual strlen of buf
* may be less than len
*/
- return buf;
+ for (int i = (int)len - 1; i >= 0; i--) {
+ if (buf[i] == '\0')
+ return buf;
+ }
}
free(buf);
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v1] perf header: Ensure read strings are '\0' terminated
2026-04-14 20:57 [PATCH v1] perf header: Ensure read strings are '\0' terminated Ian Rogers
@ 2026-04-14 21:15 ` David Laight
2026-04-14 21:21 ` David Laight
1 sibling, 0 replies; 3+ messages in thread
From: David Laight @ 2026-04-14 21:15 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
James Clark, Swapnil Sapkal, Anubhav Shelat, linux-perf-users,
linux-kernel
On Tue, 14 Apr 2026 13:57:25 -0700
Ian Rogers <irogers@google.com> wrote:
> Sashiko reviews were complaining do_read_string didn't necessarily
> ensure strings were correctly terminated. Add checking for this and if
> a string isn't correctly terminated return NULL.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/header.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index f30e48eb3fc3..fa4f6d773874 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -269,6 +269,9 @@ static char *do_read_string(struct feat_fd *ff)
> if (do_read_u32(ff, &len))
> return NULL;
>
> + if (len == 0)
> + return NULL;
> +
> buf = malloc(len);
> if (!buf)
> return NULL;
> @@ -279,7 +282,10 @@ static char *do_read_string(struct feat_fd *ff)
> * thus the actual strlen of buf
> * may be less than len
> */
> - return buf;
> + for (int i = (int)len - 1; i >= 0; i--) {
> + if (buf[i] == '\0')
> + return buf;
> + }
Would be a lot simpler to malloc(len + 1) and set buf[len] = 0.
david
> }
>
> free(buf);
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v1] perf header: Ensure read strings are '\0' terminated
2026-04-14 20:57 [PATCH v1] perf header: Ensure read strings are '\0' terminated Ian Rogers
2026-04-14 21:15 ` David Laight
@ 2026-04-14 21:21 ` David Laight
1 sibling, 0 replies; 3+ messages in thread
From: David Laight @ 2026-04-14 21:21 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
James Clark, Swapnil Sapkal, Anubhav Shelat, linux-perf-users,
linux-kernel
On Tue, 14 Apr 2026 13:57:25 -0700
Ian Rogers <irogers@google.com> wrote:
> Sashiko reviews were complaining do_read_string didn't necessarily
> ensure strings were correctly terminated. Add checking for this and if
> a string isn't correctly terminated return NULL.
Actually I'd be just as worried about corrupt data giving insane lengths
(eg if it processes some ascii text).
For that you want a better error than just 'return NULL'.
David
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/header.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index f30e48eb3fc3..fa4f6d773874 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -269,6 +269,9 @@ static char *do_read_string(struct feat_fd *ff)
> if (do_read_u32(ff, &len))
> return NULL;
>
> + if (len == 0)
> + return NULL;
> +
> buf = malloc(len);
> if (!buf)
> return NULL;
> @@ -279,7 +282,10 @@ static char *do_read_string(struct feat_fd *ff)
> * thus the actual strlen of buf
> * may be less than len
> */
> - return buf;
> + for (int i = (int)len - 1; i >= 0; i--) {
> + if (buf[i] == '\0')
> + return buf;
> + }
> }
>
> free(buf);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-14 21:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-14 20:57 [PATCH v1] perf header: Ensure read strings are '\0' terminated Ian Rogers
2026-04-14 21:15 ` David Laight
2026-04-14 21:21 ` David Laight
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox