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;
+		}
 	}
 
 	free(buf);
-- 
2.54.0.rc0.605.g598a273b03-goog

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

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