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 <[email protected]>

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/[email protected]?part=8

Reply via email to