lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250918-can-fix-mtu-v1-0-471edb942295@kernel.org>
Date: Thu, 18 Sep 2025 21:59:10 +0900
From: Vincent Mailhol <mailhol@...nel.org>
To: Oliver Hartkopp <socketcan@...tkopp.net>, 
 Marc Kleine-Budde <mkl@...gutronix.de>
Cc: linux-can@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Vincent Mailhol <mailhol@...nel.org>
Subject: [PATCH RFC 0/5] can: rework the CAN MTU logic



---
RFC preface:

I am sending this as an RFC because I am not done 100% with my testing
and this depends on the other series I just sent earlier today:

Link: https://lore.kernel.org/linux-can/20250918-can-fix-mtu-v1-0-0d1cada9393b@kernel.org/

I will drop the RFC tag once the above series is merged and appears in
net-next.
---

The CAN MTU logic is currently broken. can_change_mtu() will update
both the MTU and the CAN_CTRLMODE_FD flag.

Back then, commit bc05a8944a34 ("can: allow to change the device mtu
for CAN FD capable devices") stated that:

  The configuration can be done either with the 'fd { on | off }'
  option in the 'ip' tool from iproute2 or by setting the CAN
  netdevice MTU to CAN_MTU (16) or to CANFD_MTU (72).

  Link: https://git.kernel.org/torvalds/c/bc05a8944a34

The problem is that after doing for example:

  $ ip link set can0 mtu 72 bittiming 500000

on a CAN FD interface, we are left with a device on which CAN FD is
enabled but which does not have the FD databittiming parameters
configured.

The same goes on when setting the mtu back to 16:

  ip link set can0 type can bitrate 500000 fd on dbitrate 5000000
  ip link set can0 mtu 16

The device is now in Classical CAN mode but iproute2 is still
reporting the databittiming values (although this time, the issue
seems less critical as it is only a reporting problem).

The only way to resolve the problem and bring the device back to a
coherent state is to call again the netlink interface using the
"fd on" or "fd off" options.

The idea of being able to infer the CAN_CTRLMODE_FD flag from the MTU
value is just incorrect for physical devices. Note that this logic
remains valid on virtual interfaces (vcan and vxcan) because those do
not have control mode flags and thus no conflict occurs.

This series reworks the CAN MTU logic. The goal is to always maintain
a coherent state between the MTU and the control mode flags as listed
in below table:

		fd off, xl off		fd on, xl off		fd any, xl on
  ---------------------------------------------------------------------------
  default mtu	CAN_MTU			CANFD_MTU		CANXL_MTU
  min mtu	CAN_MTU			CANFD_MTU		CANXL_MIN_MTU
  max mtu	CAN_MTU			CANFD_MTU		CANXL_MAX_MTU

In order to switch between one column to another, the user must use
the fd/xl on/off flags. Directly modifying the MTU from one column to
the other is not permitted any more.

The CAN XL is not yet supported at the moment, so the last column is
just given as a reference to better understand what is coming up. This
series will just implement the first two columns.

While doing the rewrite, the logic is adjusted to reuse as much as
possible the net core infrastructure. By populating:

  net_device->min_mtu

and

  net_device->max_mtu

the net core infrastructure will automatically:
    
  1. validate that the user's inputs are in range.

  2. report those min and max MTU values through the netlink
     interface.
    
Point 1. allows us to get rid of the can_change_mtu() for all the
physical devices and point 2. allows the end user to see the valid MTU
range by doing a:

  $ ip --details link show can0

Finally, because using the net core, it will be possible to modify the
MTU while the device is up. As stated previously, the only
modifications allowed will be within the MTU range of a given MTU. So
for Classical CAN and CAN FD, the MTU is fixed to, respectively,
CAN_MTU and CANFD_MTU. For the upcoming CAN XL, the user will be able
to change the MTU to anything between CANXL_MIN_MTU and CANXL_MAX_MTU
even if the device is up.

The first patch of this series annotates the read access on
net_device->mtu. This preparation is needed to prevent any race
condition to occur when modifying the MTU while the device is up.

The second patch is another preparation change which moves
can_set_static_ctrlmode() from dev.h to dev.c.

The third patch populates the MTU minimum and maximum value.

The fourth patch is just a clean-up to remove the old
can_change_mtu().

The fifth and last patch comes as a bonus content and modifies the
default MTU of the vcan and vxcan so that CAN XL is on by default.

To: Oliver Hartkopp <socketcan@...tkopp.net>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: linux-can@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Signed-off-by: Vincent Mailhol <mailhol@...nel.org>

---
Vincent Mailhol (5):
      can: annotate mtu accesses with READ_ONCE()
      can: dev: turn can_set_static_ctrlmode() into a non-inline function
      can: populate the minimum and maximum MTU values
      can: treewide: remove can_change_mtu()
      can: enable CAN XL for virtual CAN devices by default

 drivers/net/can/at91_can.c                         |  1 -
 drivers/net/can/bxcan.c                            |  1 -
 drivers/net/can/c_can/c_can_main.c                 |  1 -
 drivers/net/can/can327.c                           |  1 -
 drivers/net/can/cc770/cc770.c                      |  1 -
 drivers/net/can/ctucanfd/ctucanfd_base.c           |  1 -
 drivers/net/can/dev/dev.c                          | 54 +++++++++++-----------
 drivers/net/can/dev/netlink.c                      |  9 ++--
 drivers/net/can/esd/esd_402_pci-core.c             |  1 -
 drivers/net/can/flexcan/flexcan-core.c             |  1 -
 drivers/net/can/grcan.c                            |  1 -
 drivers/net/can/ifi_canfd/ifi_canfd.c              |  1 -
 drivers/net/can/janz-ican3.c                       |  1 -
 drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c |  1 -
 drivers/net/can/m_can/m_can.c                      |  1 -
 drivers/net/can/mscan/mscan.c                      |  1 -
 drivers/net/can/peak_canfd/peak_canfd.c            |  1 -
 drivers/net/can/rcar/rcar_can.c                    |  1 -
 drivers/net/can/rcar/rcar_canfd.c                  |  1 -
 drivers/net/can/rockchip/rockchip_canfd-core.c     |  1 -
 drivers/net/can/sja1000/sja1000.c                  |  1 -
 drivers/net/can/slcan/slcan-core.c                 |  1 -
 drivers/net/can/softing/softing_main.c             |  1 -
 drivers/net/can/spi/hi311x.c                       |  1 -
 drivers/net/can/spi/mcp251x.c                      |  1 -
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c     |  1 -
 drivers/net/can/sun4i_can.c                        |  1 -
 drivers/net/can/ti_hecc.c                          |  1 -
 drivers/net/can/usb/ems_usb.c                      |  1 -
 drivers/net/can/usb/esd_usb.c                      |  1 -
 drivers/net/can/usb/etas_es58x/es58x_core.c        |  1 -
 drivers/net/can/usb/f81604.c                       |  1 -
 drivers/net/can/usb/gs_usb.c                       |  1 -
 drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c   |  1 -
 drivers/net/can/usb/mcba_usb.c                     |  1 -
 drivers/net/can/usb/peak_usb/pcan_usb_core.c       |  1 -
 drivers/net/can/usb/ucan.c                         |  1 -
 drivers/net/can/usb/usb_8dev.c                     |  1 -
 drivers/net/can/vcan.c                             |  2 +-
 drivers/net/can/vxcan.c                            |  2 +-
 drivers/net/can/xilinx_can.c                       |  1 -
 include/linux/can/dev.h                            | 25 ++--------
 net/can/af_can.c                                   |  2 +-
 net/can/isotp.c                                    |  2 +-
 net/can/raw.c                                      |  2 +-
 45 files changed, 38 insertions(+), 97 deletions(-)
---
base-commit: 5e87fdc37f8dc619549d49ba5c951b369ce7c136
change-id: 20250915-can-fix-mtu-050a94b563a0
prerequisite-change-id: 20250918-can-fix-mtu-b521e1ed1a29:v1
prerequisite-patch-id: ace4f8ad663553a2a04ec56ca28fd2445b0ce06e
prerequisite-patch-id: 10ebdab5a42d989075b951bba3e93b5bea178ead
prerequisite-patch-id: 11502adb20b40227221185c363cd4f0733ca2239
prerequisite-patch-id: be9ed4f4caaa323a1757a7c5d4d2a4bfb2cfc124

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ