Linux CAN drivers development
 help / color / mirror / Atom feed
* [PATCH net] can: netlink: fix CAN XL PWM attribute sizing
@ 2026-09-04 15:26 Zhu Ling
  2026-09-04 15:40 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Zhu Ling @ 2026-09-04 15:26 UTC (permalink / raw)
  To: linux-can; +Cc: mkl, mailhol, socketcan, linux-kernel, stable, Zhu Ling

can_pwm_fill_info() emits the PWM capability limits whenever pwm_const is
present, regardless of whether TMS is enabled. In contrast,
can_pwm_get_size() currently returns zero when TMS is disabled. It
therefore underestimates the required netlink message size by the nested
attribute header and six u32 attributes.

Always account for the PWM capability attributes when pwm_const is
present, and account for the three runtime PWM values only when TMS is
enabled.

Fixes: 46552323fa67 ("can: netlink: add PWM netlink interface")
Cc: stable@vger.kernel.org
Signed-off-by: Zhu Ling <zhuling0805@qq.com>
---
 drivers/net/can/dev/netlink.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 766d455950f5..0eac5a3f577e 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -780,7 +780,7 @@ static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
 {
 	size_t size;
 
-	if (!pwm_const || !pwm_on)
+	if (!pwm_const)
 		return 0;
 
 	size = nla_total_size(0);			/* nest IFLA_CAN_PWM */
@@ -792,9 +792,11 @@ static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
 	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO_MIN */
 	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO_MAX */
 
-	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMS */
-	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWML */
-	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO */
+	if (pwm_on) {
+		size += nla_total_size(sizeof(u32));	/* IFLA_CAN_PWM_PWMS */
+		size += nla_total_size(sizeof(u32));	/* IFLA_CAN_PWM_PWML */
+		size += nla_total_size(sizeof(u32));	/* IFLA_CAN_PWM_PWMO */
+	}
 
 	return size;
 }

base-commit: 641d03105cc0d2437e32fdeec164f91a4ccef6c4
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] can: netlink: fix CAN XL PWM attribute sizing
  2026-09-04 15:26 [PATCH net] can: netlink: fix CAN XL PWM attribute sizing Zhu Ling
@ 2026-09-04 15:40 ` sashiko-bot
  2026-09-04 17:08   ` zhuling0805
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-09-04 15:40 UTC (permalink / raw)
  To: Zhu Ling
  Cc: Vincent Mailhol, Oleksij Rempel, Marc Kleine-Budde, linux-can,
	Oliver Hartkopp

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

Pre-existing issues:
- [High] can_get_size() underestimates the netlink message size by not using nla_total_size() for the IFLA_CAN_BITRATE_MAX attribute.
--

commit 65a98e250b77c16c65b20dbe5d32fe88f1c06e1b
Author: Zhu Ling <zhuling0805@qq.com>

can: netlink: fix CAN XL PWM attribute sizing

can_pwm_fill_info() emits the PWM capability limits whenever pwm_const is
present, regardless of whether TMS is enabled. In contrast,
can_pwm_get_size() currently returns zero when TMS is disabled. It
therefore underestimates the required netlink message size by the nested
attribute header and six u32 attributes.

Always account for the PWM capability attributes when pwm_const is
present, and account for the three runtime PWM values only when TMS is
enabled.

> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 766d455950f55..0eac5a3f577ec 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -780,7 +780,7 @@ static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
>  {
>  	size_t size;
>  
> -	if (!pwm_const || !pwm_on)
> +	if (!pwm_const)
>  		return 0;
>  
>  	size = nla_total_size(0);			/* nest IFLA_CAN_PWM */

[ ... ]

> @@ -792,9 +792,11 @@ static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
>  	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO_MIN */
>  	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO_MAX */
>  
> -	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMS */
> -	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWML */
> -	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO */
> +	if (pwm_on) {
> +		size += nla_total_size(sizeof(u32));	/* IFLA_CAN_PWM_PWMS */
> +		size += nla_total_size(sizeof(u32));	/* IFLA_CAN_PWM_PWML */
> +		size += nla_total_size(sizeof(u32));	/* IFLA_CAN_PWM_PWMO */
> +	}
>  
>  	return size;
>  }

[Severity: High]
This is a pre-existing issue, but while looking at the netlink message size
calculations, I noticed a potential under-allocation in the caller:

drivers/net/can/dev/netlink.c:can_get_size() {
	...
	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 += can_ctrlmode_ext_get_size();			/* IFLA_CAN_CTRLMODE_EXT */
	...
}

Does the IFLA_CAN_BITRATE_MAX size calculation underestimate the required
netlink message size? It appears to add only the payload size instead of
wrapping it in nla_total_size() to account for the NLA header.

If the allocation doesn't happen to have enough slack to cover the calculation
error, couldn't this cause userspace netlink requests (like `ip link show`) to
randomly fail with -EMSGSIZE and truncate the response?

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] can: netlink: fix CAN XL PWM attribute sizing
  2026-09-04 15:40 ` sashiko-bot
@ 2026-09-04 17:08   ` zhuling0805
  0 siblings, 0 replies; 3+ messages in thread
From: zhuling0805 @ 2026-09-04 17:08 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Vincent Mailhol, Oleksij Rempel, Marc Kleine-Budde, linux-can,
	Oliver Hartkopp

Thanks for the review.

Agreed. The IFLA_CAN_BITRATE_MAX calculation is a pre-existing issue
independent of the PWM sizing fix. I have addressed it in a separate
patch:

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

The PWM patch itself remains unchanged.

Regards,
Zhu



原始邮件
发件人:sashiko-bot <sashiko-bot@kernel.org>
发件时间:2026年9月4日 23:40
收件人:Zhu Ling <zhuling0805@qq.com>
抄送:Vincent Mailhol <mailhol@kernel.org>, Oleksij Rempel <o.rempel@pengutronix.de>, Marc Kleine-Budde <mkl@pengutronix.de>, linux-can <linux-can@vger.kernel.org>, Oliver Hartkopp <socketcan@hartkopp.net>
主题:Re: [PATCH net] can: netlink: fix CAN XL PWM attribute sizing


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

Pre-existing issues:
- [High] can_get_size() underestimates the netlink message size by not using nla_total_size() for the IFLA_CAN_BITRATE_MAX attribute.
--

commit 65a98e250b77c16c65b20dbe5d32fe88f1c06e1b
Author: Zhu Ling <zhuling0805@qq.com>

can: netlink: fix CAN XL PWM attribute sizing

can_pwm_fill_info() emits the PWM capability limits whenever pwm_const is
present, regardless of whether TMS is enabled. In contrast,
can_pwm_get_size() currently returns zero when TMS is disabled. It
therefore underestimates the required netlink message size by the nested
attribute header and six u32 attributes.

Always account for the PWM capability attributes when pwm_const is
present, and account for the three runtime PWM values only when TMS is
enabled.

> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 766d455950f55..0eac5a3f577ec 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -780,7 +780,7 @@ static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
>  {
>   size_t size;
>  
> - if (!pwm_const || !pwm_on)
> + if (!pwm_const)
>   return 0;
>  
>   size = nla_total_size(0); /* nest IFLA_CAN_PWM */

[ ... ]

> @@ -792,9 +792,11 @@ static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
>   size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWMO_MIN */
>   size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWMO_MAX */
>  
> - size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWMS */
> - size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWML */
> - size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWMO */
> + if (pwm_on) {
> + size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWMS */
> + size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWML */
> + size += nla_total_size(sizeof(u32)); /* IFLA_CAN_PWM_PWMO */
> + }
>  
>   return size;
>  }

[Severity: High]
This is a pre-existing issue, but while looking at the netlink message size
calculations, I noticed a potential under-allocation in the caller:

drivers/net/can/dev/netlink.c:can_get_size() {
...
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 += can_ctrlmode_ext_get_size(); /* IFLA_CAN_CTRLMODE_EXT */
...
}

Does the IFLA_CAN_BITRATE_MAX size calculation underestimate the required
netlink message size? It appears to add only the payload size instead of
wrapping it in nla_total_size() to account for the NLA header.

If the allocation doesn't happen to have enough slack to cover the calculation
error, couldn't this cause userspace netlink requests (like `ip link show`) to
randomly fail with -EMSGSIZE and truncate the response?

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-04 17:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04 15:26 [PATCH net] can: netlink: fix CAN XL PWM attribute sizing Zhu Ling
2026-09-04 15:40 ` sashiko-bot
2026-09-04 17:08   ` zhuling0805

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox