mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow
@ 2025-09-18  9:00 Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 1/4] can: etas_es58x: " Vincent Mailhol
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Vincent Mailhol @ 2025-09-18  9:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Arunachalam Santhanam, Akshay Bhat, Wolfgang Grandegger,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Ripard,
	Gerhard Bertelsmann, Yasushi SHOJI,
	Remigiusz Kołłątaj, linux-can, linux-kernel,
	linux-arm-kernel, linux-sunxi, Vincent Mailhol

Four drivers, namely etas_es58x, hi311x, sun4i_can and mcba_usb forgot
to populate their net_device_ops->ndo_change_mtu(). Because of that,
the user is free to configure any MTU on these interfaces.

This can be abused by an attacker who could craft some skbs and send
them through PF_PACKET to perform a buffer overflow of up to 247 bytes
in each of these drivers.

This series contains four patches, one for each of the drivers, to add
the missing ndo_change_mtu() callback. The descriptions contain
detailed explanations of how the buffer overflow could be triggered.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Vincent Mailhol (4):
      can: etas_es58x: populate ndo_change_mtu() to prevent buffer overflow
      can: hi311x: populate ndo_change_mtu() to prevent buffer overflow
      can: sun4i_can: populate ndo_change_mtu() to prevent buffer overflow
      can: mcba_usb: populate ndo_change_mtu() to prevent buffer overflow

 drivers/net/can/spi/hi311x.c                | 1 +
 drivers/net/can/sun4i_can.c                 | 1 +
 drivers/net/can/usb/etas_es58x/es58x_core.c | 3 ++-
 drivers/net/can/usb/mcba_usb.c              | 1 +
 4 files changed, 5 insertions(+), 1 deletion(-)
---
base-commit: f83ec76bf285bea5727f478a68b894f5543ca76e
change-id: 20250918-can-fix-mtu-b521e1ed1a29

Best regards,
-- 
Vincent Mailhol <mailhol@kernel.org>


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

* [PATCH 1/4] can: etas_es58x: populate ndo_change_mtu() to prevent buffer overflow
  2025-09-18  9:00 [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow Vincent Mailhol
@ 2025-09-18  9:00 ` Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 2/4] can: hi311x: " Vincent Mailhol
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol @ 2025-09-18  9:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Arunachalam Santhanam, Akshay Bhat, Wolfgang Grandegger,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Ripard,
	Gerhard Bertelsmann, Yasushi SHOJI,
	Remigiusz Kołłątaj, linux-can, linux-kernel,
	linux-arm-kernel, linux-sunxi, Vincent Mailhol

Sending an PF_PACKET allows to bypass the CAN framework logic and to
directly reach the xmit() function of a CAN driver. The only check
which is performed by the PF_PACKET framework is to make sure that
skb->len fits the interface's MTU.

Unfortunately, because the etas_es58x driver does not populate its
net_device_ops->ndo_change_mtu(), it is possible for an attacker to
configure an invalid MTU by doing, for example:

  $ ip link set can0 mtu 9999

After doing so, the attacker could open a PF_PACKET socket using the
ETH_P_CANXL protocol:

	socket(PF_PACKET, SOCK_RAW, htons(ETH_P_CANXL));

to inject a malicious CAN XL frames. For example:

	struct canxl_frame frame = {
		.flags = 0xff,
		.len = 2048,
	};

The CAN drivers' xmit() function are calling can_dev_dropped_skb() to
check that the skb is valid, unfortunately under above conditions, the
malicious packet is able to go through can_dev_dropped_skb() checks:

  1. the skb->protocol is set to ETH_P_CANXL which is valid (the
     function does not check the actual device capabilities).

  2. the length is a valid CAN XL length.

And so, es58x_start_xmit() receives a CAN XL frame which it is not
able to correctly handle and will thus misinterpret it as a CAN(FD)
frame.

This can result in a buffer overflow. For example, using the es581.4
variant, the frame will be dispatched to es581_4_tx_can_msg(), go
through the last check at the beginning of this function:

	if (can_is_canfd_skb(skb))
		return -EMSGSIZE;

and reach this line:

	memcpy(tx_can_msg->data, cf->data, cf->len);

Here, cf->len corresponds to the flags field of the CAN XL frame. In
our previous example, we set canxl_frame->flags to 0xff. Because the
maximum expected length is 8, a buffer overflow of 247 bytes occurs!

Populate net_device_ops->ndo_change_mtu() to ensure that the
interface's MTU can not be set to anything bigger than CAN_MTU or
CANFD_MTU (depending on the device capabilities). By fixing the root
cause, this prevents the buffer overflow.

Fixes: 8537257874e9 ("can: etas_es58x: add core support for ETAS ES58X CAN USB interfaces")
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/usb/etas_es58x/es58x_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.c b/drivers/net/can/usb/etas_es58x/es58x_core.c
index db1acf6d504cf37cec724d392c9f7fee2d1f6045..adc91873c083f968ff7571a5b0db5747988482c5 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_core.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
@@ -7,7 +7,7 @@
  *
  * Copyright (c) 2019 Robert Bosch Engineering and Business Solutions. All rights reserved.
  * Copyright (c) 2020 ETAS K.K.. All rights reserved.
- * Copyright (c) 2020-2022 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
+ * Copyright (c) 2020-2025 Vincent Mailhol <mailhol@kernel.org>
  */
 
 #include <linux/unaligned.h>
@@ -1977,6 +1977,7 @@ static const struct net_device_ops es58x_netdev_ops = {
 	.ndo_stop = es58x_stop,
 	.ndo_start_xmit = es58x_start_xmit,
 	.ndo_eth_ioctl = can_eth_ioctl_hwts,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct ethtool_ops es58x_ethtool_ops = {

-- 
2.49.1


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

* [PATCH 2/4] can: hi311x: populate ndo_change_mtu() to prevent buffer overflow
  2025-09-18  9:00 [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 1/4] can: etas_es58x: " Vincent Mailhol
@ 2025-09-18  9:00 ` Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 3/4] can: sun4i_can: " Vincent Mailhol
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol @ 2025-09-18  9:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Arunachalam Santhanam, Akshay Bhat, Wolfgang Grandegger,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Ripard,
	Gerhard Bertelsmann, Yasushi SHOJI,
	Remigiusz Kołłątaj, linux-can, linux-kernel,
	linux-arm-kernel, linux-sunxi, Vincent Mailhol

Sending an PF_PACKET allows to bypass the CAN framework logic and to
directly reach the xmit() function of a CAN driver. The only check
which is performed by the PF_PACKET framework is to make sure that
skb->len fits the interface's MTU.

Unfortunately, because the sun4i_can driver does not populate its
net_device_ops->ndo_change_mtu(), it is possible for an attacker to
configure an invalid MTU by doing, for example:

  $ ip link set can0 mtu 9999

After doing so, the attacker could open a PF_PACKET socket using the
ETH_P_CANXL protocol:

	socket(PF_PACKET, SOCK_RAW, htons(ETH_P_CANXL))

to inject a malicious CAN XL frames. For example:

	struct canxl_frame frame = {
		.flags = 0xff,
		.len = 2048,
	};

The CAN drivers' xmit() function are calling can_dev_dropped_skb() to
check that the skb is valid, unfortunately under above conditions, the
malicious packet is able to go through can_dev_dropped_skb() checks:

  1. the skb->protocol is set to ETH_P_CANXL which is valid (the
     function does not check the actual device capabilities).

  2. the length is a valid CAN XL length.

And so, hi3110_hard_start_xmit() receives a CAN XL frame which it is
not able to correctly handle and will thus misinterpret it as a CAN
frame. The driver will consume frame->len as-is with no further
checks.

This can result in a buffer overflow later on in hi3110_hw_tx() on
this line:

	memcpy(buf + HI3110_FIFO_EXT_DATA_OFF,
	       frame->data, frame->len);

Here, frame->len corresponds to the flags field of the CAN XL frame.
In our previous example, we set canxl_frame->flags to 0xff. Because
the maximum expected length is 8, a buffer overflow of 247 bytes
occurs!

Populate net_device_ops->ndo_change_mtu() to ensure that the
interface's MTU can not be set to anything bigger than CAN_MTU. By
fixing the root cause, this prevents the buffer overflow.

Fixes: 57e83fb9b746 ("can: hi311x: Add Holt HI-311x CAN driver")
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/spi/hi311x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c
index 09ae218315d73d49c2ec4280707911966fce1c31..6441ff3b4198718e8e662daa3022f06a2405f322 100644
--- a/drivers/net/can/spi/hi311x.c
+++ b/drivers/net/can/spi/hi311x.c
@@ -812,6 +812,7 @@ static const struct net_device_ops hi3110_netdev_ops = {
 	.ndo_open = hi3110_open,
 	.ndo_stop = hi3110_stop,
 	.ndo_start_xmit = hi3110_hard_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct ethtool_ops hi3110_ethtool_ops = {

-- 
2.49.1


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

* [PATCH 3/4] can: sun4i_can: populate ndo_change_mtu() to prevent buffer overflow
  2025-09-18  9:00 [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 1/4] can: etas_es58x: " Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 2/4] can: hi311x: " Vincent Mailhol
@ 2025-09-18  9:00 ` Vincent Mailhol
  2025-09-18  9:00 ` [PATCH 4/4] can: mcba_usb: " Vincent Mailhol
  2025-09-19 17:04 ` [PATCH 0/4] can: " Marc Kleine-Budde
  4 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol @ 2025-09-18  9:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Arunachalam Santhanam, Akshay Bhat, Wolfgang Grandegger,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Ripard,
	Gerhard Bertelsmann, Yasushi SHOJI,
	Remigiusz Kołłątaj, linux-can, linux-kernel,
	linux-arm-kernel, linux-sunxi, Vincent Mailhol

Sending an PF_PACKET allows to bypass the CAN framework logic and to
directly reach the xmit() function of a CAN driver. The only check
which is performed by the PF_PACKET framework is to make sure that
skb->len fits the interface's MTU.

Unfortunately, because the sun4i_can driver does not populate its
net_device_ops->ndo_change_mtu(), it is possible for an attacker to
configure an invalid MTU by doing, for example:

  $ ip link set can0 mtu 9999

After doing so, the attacker could open a PF_PACKET socket using the
ETH_P_CANXL protocol:

	socket(PF_PACKET, SOCK_RAW, htons(ETH_P_CANXL))

to inject a malicious CAN XL frames. For example:

	struct canxl_frame frame = {
		.flags = 0xff,
		.len = 2048,
	};

The CAN drivers' xmit() function are calling can_dev_dropped_skb() to
check that the skb is valid, unfortunately under above conditions, the
malicious packet is able to go through can_dev_dropped_skb() checks:

  1. the skb->protocol is set to ETH_P_CANXL which is valid (the
     function does not check the actual device capabilities).

  2. the length is a valid CAN XL length.

And so, sun4ican_start_xmit() receives a CAN XL frame which it is not
able to correctly handle and will thus misinterpret it as a CAN frame.

This can result in a buffer overflow. The driver will consume cf->len
as-is with no further checks on this line:

	dlc = cf->len;

Here, cf->len corresponds to the flags field of the CAN XL frame. In
our previous example, we set canxl_frame->flags to 0xff. Because the
maximum expected length is 8, a buffer overflow of 247 bytes occurs a
couple line below when doing:

	for (i = 0; i < dlc; i++)
		writel(cf->data[i], priv->base + (dreg + i * 4));

Populate net_device_ops->ndo_change_mtu() to ensure that the
interface's MTU can not be set to anything bigger than CAN_MTU. By
fixing the root cause, this prevents the buffer overflow.

Fixes: 0738eff14d81 ("can: Allwinner A10/A20 CAN Controller support - Kernel module")
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/sun4i_can.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
index 6fcb301ef611d0c8bdb8720aaa77dc78950123d6..53bfd873de9bdecaf6923049007f9efd71289dd3 100644
--- a/drivers/net/can/sun4i_can.c
+++ b/drivers/net/can/sun4i_can.c
@@ -768,6 +768,7 @@ static const struct net_device_ops sun4ican_netdev_ops = {
 	.ndo_open = sun4ican_open,
 	.ndo_stop = sun4ican_close,
 	.ndo_start_xmit = sun4ican_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct ethtool_ops sun4ican_ethtool_ops = {

-- 
2.49.1


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

* [PATCH 4/4] can: mcba_usb: populate ndo_change_mtu() to prevent buffer overflow
  2025-09-18  9:00 [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow Vincent Mailhol
                   ` (2 preceding siblings ...)
  2025-09-18  9:00 ` [PATCH 3/4] can: sun4i_can: " Vincent Mailhol
@ 2025-09-18  9:00 ` Vincent Mailhol
  2025-09-19 17:04 ` [PATCH 0/4] can: " Marc Kleine-Budde
  4 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol @ 2025-09-18  9:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Arunachalam Santhanam, Akshay Bhat, Wolfgang Grandegger,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Ripard,
	Gerhard Bertelsmann, Yasushi SHOJI,
	Remigiusz Kołłątaj, linux-can, linux-kernel,
	linux-arm-kernel, linux-sunxi, Vincent Mailhol

Sending an PF_PACKET allows to bypass the CAN framework logic and to
directly reach the xmit() function of a CAN driver. The only check
which is performed by the PF_PACKET framework is to make sure that
skb->len fits the interface's MTU.

Unfortunately, because the mcba_usb driver does not populate its
net_device_ops->ndo_change_mtu(), it is possible for an attacker to
configure an invalid MTU by doing, for example:

  $ ip link set can0 mtu 9999

After doing so, the attacker could open a PF_PACKET socket using the
ETH_P_CANXL protocol:

	socket(PF_PACKET, SOCK_RAW, htons(ETH_P_CANXL))

to inject a malicious CAN XL frames. For example:

	struct canxl_frame frame = {
		.flags = 0xff,
		.len = 2048,
	};

The CAN drivers' xmit() function are calling can_dev_dropped_skb() to
check that the skb is valid, unfortunately under above conditions, the
malicious packet is able to go through can_dev_dropped_skb() checks:

  1. the skb->protocol is set to ETH_P_CANXL which is valid (the
     function does not check the actual device capabilities).

  2. the length is a valid CAN XL length.

And so, mcba_usb_start_xmit() receives a CAN XL frame which it is not
able to correctly handle and will thus misinterpret it as a CAN frame.

This can result in a buffer overflow. The driver will consume cf->len
as-is with no further checks on these lines:

	usb_msg.dlc = cf->len;

	memcpy(usb_msg.data, cf->data, usb_msg.dlc);

Here, cf->len corresponds to the flags field of the CAN XL frame. In
our previous example, we set canxl_frame->flags to 0xff. Because the
maximum expected length is 8, a buffer overflow of 247 bytes occurs!

Populate net_device_ops->ndo_change_mtu() to ensure that the
interface's MTU can not be set to anything bigger than CAN_MTU. By
fixing the root cause, this prevents the buffer overflow.

Fixes: 51f3baad7de9 ("can: mcba_usb: Add support for Microchip CAN BUS Analyzer")
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/usb/mcba_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index 41c0a1c399bf36104fbf8991b537acdb01e7e950..1f9b915094e64de875adff35026e9cd3beb0410d 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -761,6 +761,7 @@ static const struct net_device_ops mcba_netdev_ops = {
 	.ndo_open = mcba_usb_open,
 	.ndo_stop = mcba_usb_close,
 	.ndo_start_xmit = mcba_usb_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct ethtool_ops mcba_ethtool_ops = {

-- 
2.49.1


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

* Re: [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow
  2025-09-18  9:00 [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow Vincent Mailhol
                   ` (3 preceding siblings ...)
  2025-09-18  9:00 ` [PATCH 4/4] can: mcba_usb: " Vincent Mailhol
@ 2025-09-19 17:04 ` Marc Kleine-Budde
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-09-19 17:04 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Oliver Hartkopp, Arunachalam Santhanam, Akshay Bhat,
	Wolfgang Grandegger, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Maxime Ripard, Gerhard Bertelsmann,
	Yasushi SHOJI, Remigiusz Kołłątaj, linux-can,
	linux-kernel, linux-arm-kernel, linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

On 18.09.2025 18:00:23, Vincent Mailhol wrote:
> Four drivers, namely etas_es58x, hi311x, sun4i_can and mcba_usb forgot
> to populate their net_device_ops->ndo_change_mtu(). Because of that,
> the user is free to configure any MTU on these interfaces.
> 
> This can be abused by an attacker who could craft some skbs and send
> them through PF_PACKET to perform a buffer overflow of up to 247 bytes
> in each of these drivers.
> 
> This series contains four patches, one for each of the drivers, to add
> the missing ndo_change_mtu() callback. The descriptions contain
> detailed explanations of how the buffer overflow could be triggered.
> 
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>

Added to linux-can.

Thanks,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2025-09-19 17:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-18  9:00 [PATCH 0/4] can: populate ndo_change_mtu() to prevent buffer overflow Vincent Mailhol
2025-09-18  9:00 ` [PATCH 1/4] can: etas_es58x: " Vincent Mailhol
2025-09-18  9:00 ` [PATCH 2/4] can: hi311x: " Vincent Mailhol
2025-09-18  9:00 ` [PATCH 3/4] can: sun4i_can: " Vincent Mailhol
2025-09-18  9:00 ` [PATCH 4/4] can: mcba_usb: " Vincent Mailhol
2025-09-19 17:04 ` [PATCH 0/4] can: " Marc Kleine-Budde

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

all inboxes | Powered by JetHome®