Re: [PATCH v1] perf header: Ensure read strings are '\0' terminated

[David Laight <david.laight.linux@gmail.com>]

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