Re: [PATCH] lib/igt_amd: don't rely on NUL separators parsing link_settings

[Alex Hung <alex.hung@amd.com>]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 6/23/26 09:55, Harry Wentland wrote:
> igt_amd_read_link_settings() parsed the dp_link_settings debugfs node by
> replacing the NUL bytes that the kernel accidentally wrote between the
> Current/Verified/Reported/Preferred records with ';' and using those as
> strtok delimiters.
> 
> The kernel side has been fixed to emit a single, properly NUL-terminated
> string with no embedded NULs (amdgpu: fix malformed link_settings debugfs
> output). With that fix the buffer no longer contains internal separators,
> so the old parser collapsed the whole string into a single token: only the
> Current record was parsed while Verified, Reported and Preferred were left
> unset (0). amd_ilr then wrote a lane count of 0 and asserted it against the
> trained lane count, failing the ilr-link-training-configs subtest. The
> NUL-scanning while loop also walked into uninitialized stack.
> 
> Parse the node by locating each label and reading the three values that
> follow it, so it no longer depends on embedded NULs or any other record
> separator and works with the contiguous string the kernel now produces.
> 
> Assisted-by: Copilot:claude-opus-4.8
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> ---
>   lib/igt_amd.c | 38 ++++++++++++++++++++------------------
>   1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/igt_amd.c b/lib/igt_amd.c
> index 47e587e54de0..7d33ab65858c 100644
> --- a/lib/igt_amd.c
> +++ b/lib/igt_amd.c
> @@ -833,10 +833,13 @@ int igt_amd_trigger_hotplug(int drm_fd, char *connector_name)
>   void igt_amd_read_link_settings(
>   	int drm_fd, const char *connector_name, int *lane_count, int *link_rate, int *link_spread)
>   {
> +	static const char * const labels[] = {
> +		"Current:", "Verified:", "Reported:", "Preferred:"
> +	};
>   	int fd, ret;
>   	char buf[101];
> -	int i = 0;
> -	char *token_end, *val_token;
> +	char *ptr;
> +	int i;
>   
>   	fd = igt_debugfs_connector_dir(drm_fd, connector_name, O_RDONLY);
>   	if (fd < 0) {
> @@ -850,23 +853,22 @@ void igt_amd_read_link_settings(
>   
>   	close(fd);
>   
> -	/* Between current, verified, reported, and preferred are null terminators,
> -	 * replace them with ';' to use as the delimiter for strtok. */
> -	while (strlen(buf) < sizeof(buf) - 1 && buf[strlen(buf)] == '\0')
> -		buf[strlen(buf)] = ';';
> +	/* The debugfs node returns a single NUL-terminated string of the form:
> +	 *   "Current:  %d  0x%x  %d  Verified:  %d  0x%x  %d  "
> +	 *   "Reported:  %d  0x%x  %d  Preferred:  %d  0x%x  %d\n"
> +	 * Locate each label and parse the three values that follow it. Do not
> +	 * rely on embedded NUL bytes (or any other delimiter) between records,
> +	 * as the kernel emits a single contiguous string. */
> +	ptr = buf;
> +	for (i = 0; i < 4; i++) {
> +		ptr = strstr(ptr, labels[i]);
> +		if (ptr == NULL)
> +			break;
>   
> -	/* Parse values read from file. */
> -	for (char *token = strtok_r(buf, ";", &token_end);
> -	     token != NULL;
> -	     token = strtok_r(NULL, ";", &token_end))
> -	{
> -		strtok_r(token, ": ", &val_token);
> -		lane_count[i] = strtol(val_token, &val_token, 10);
> -		link_rate[i] = strtol(val_token, &val_token, 16);
> -		link_spread[i] = strtol(val_token, &val_token, 10);
> -		i++;
> -
> -		if (i > 3) return;
> +		ptr += strlen(labels[i]);
> +		lane_count[i] = strtol(ptr, &ptr, 10);
> +		link_rate[i] = strtol(ptr, &ptr, 16);
> +		link_spread[i] = strtol(ptr, &ptr, 10);
>   	}
>   }
>