Re: [PATCH net] can: netlink: fix IFLA_CAN_BITRATE_MAX attribute sizing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing netlink payload size underestimation in `can_pwm_get_size()` when PWM capabilities exist but `CAN_CTRLMODE_XL_TMS` is disabled, leading to `-EMSGSIZE` when filling the response.
--

commit 9a3cf7f66d6ea7a9d4aba48ff7c5376d72db7fe3
Author: Zhu Ling <zhuling0805@qq.com>

can: netlink: fix IFLA_CAN_BITRATE_MAX attribute sizing

This commit fixes an issue in can_get_size() where the size for
IFLA_CAN_BITRATE_MAX was missing the netlink attribute header overhead. By
using nla_total_size(), it prevents -EMSGSIZE errors when the skb has no slack
left.

> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 766d455950f55..ba26c61a82c2f 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -822,7 +822,7 @@ static size_t can_get_size(const struct net_device *dev)
>  	if (priv->bitrate_const)				/* IFLA_CAN_BITRATE_CONST */
>  		size += nla_total_size(sizeof(*priv->bitrate_const) *
>  				       priv->bitrate_const_cnt);
> -	size += sizeof(priv->bitrate_max);			/* IFLA_CAN_BITRATE_MAX */
> +	size += nla_total_size(sizeof(priv->bitrate_max));	/* IFLA_CAN_BITRATE_MAX */
>  	size += can_ctrlmode_ext_get_size();			/* IFLA_CAN_CTRLMODE_EXT */
>  
>  	size += can_data_bittiming_get_size(&priv->fd,

[Severity: High]
This isn't a bug introduced by this patch, but while looking at
can_get_size(), I noticed another potential size underestimation just below
this diff in can_pwm_get_size():

drivers/net/can/dev/netlink.c:can_pwm_get_size() {
	if (!pwm_const || !pwm_on)
		return 0;
	...
}

If a CAN interface has hardware PWM support (pwm_const is non-NULL) but PWM is
currently disabled (pwm_on is false), can_pwm_get_size() returns 0.

However, can_pwm_fill_info() only checks pwm_const:

drivers/net/can/dev/netlink.c:can_pwm_fill_info() {
	if (!pwm_const)
		return 0;

	nest = nla_nest_start(skb, IFLA_CAN_XL_PWM);
	...
}

Could this mismatch cause can_pwm_fill_info() to write 52 bytes that were not
accounted for by can_get_size() when CAN_CTRLMODE_XL_TMS is disabled?

If so, could this also overflow the skb and cause nla_put() to fail with
-EMSGSIZE, breaking interface notifications and queries like 'ip link show'
for such devices?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_D1C873B86B0E92011A264B0E196B911D5308@qq.com?part=1