Re: [PATCH 08/28] devlink: Refactor strlcat() to seq_buf in __devlink_compat_running_version()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] devlink: data truncation in ethtool fw_version due to seq_buf_init()
--

commit 211bb0968eb0ca01b66de46e1d8900c3f0f30b64
Author: Bill Wendling <morbo@google.com>

devlink: Refactor strlcat() to seq_buf in __devlink_compat_running_version()

This commit replaces the deprecated strlcat() API in
__devlink_compat_running_version() with struct seq_buf and seq_buf_printf()
to append running firmware version strings.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/net/devlink/dev.c b/net/devlink/dev.c
> index 55959b0ff5ab4..987b071345c41 100644
> --- a/net/devlink/dev.c
> +++ b/net/devlink/dev.c
[ ... ]
> @@ -1202,6 +1204,9 @@ static void __devlink_compat_running_version(struct devlink *devlink,
>  	if (err)
>  		goto free_msg;
>  
> +	seq_buf_init(&s, buf, len);
> +	s.len = strnlen(buf, len);

[Severity: Medium]
Does this code inadvertently erase the existing contents of buf?

Looking at seq_buf_init(), it internally calls seq_buf_clear() which
unconditionally sets the first byte of the buffer to a NUL character. As a
result, the subsequent strnlen(buf, len) call will always evaluate to 0.

Since __devlink_compat_running_version() is meant to append devlink info to
the buffer, wouldn't erasing it here lose the driver's native fw_version
string that was previously populated by the driver's get_drvinfo() callback?

This could cause userspace applications calling ETHTOOL_GDRVINFO to receive
only the devlink info instead of the expected concatenated native and devlink
version string.

>  	nla_for_each_attr_type(nlattr, DEVLINK_ATTR_INFO_VERSION_RUNNING,
>  			       (void *)msg->data, msg->len, rem) {
>  		const struct nlattr *kv;
> @@ -1209,8 +1214,7 @@ static void __devlink_compat_running_version(struct devlink *devlink,
>  
>  		nla_for_each_nested_type(kv, DEVLINK_ATTR_INFO_VERSION_VALUE,
>  					 nlattr, rem_kv) {
> -			strlcat(buf, nla_data(kv), len);
> -			strlcat(buf, " ", len);
> +			seq_buf_printf(&s, "%s ", (const char *)nla_data(kv));
>  		}
>  	}
>  free_msg:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915081910.4142719-1-morbo@google.com?part=8