From 140ad8b3051df596d5d6447f062c31f244508df2 Mon Sep 17 00:00:00 2001 From: Catalin Visinescu Date: Mon, 15 Jun 2026 00:35:53 -0400 Subject: [PATCH] arch/arm/src/at32/at32_can: Division by Zero in CAN Bit Timing Commands An unchecked integer assignment in the CAN driver may result in a division-by-zero which would result in a kernel crash (denial of service). Tested locally, builds fine. An unchecked integer assignment in the CAN driver may result in a division-by-zero which would result in a kernel crash (denial of service). The CAN driver `can_ioctl` function, shown below (drivers/can/can.c), receives commands from user processes. Its received arguments `cmd` and content of `arg` are under attacker's control. In the CAN driver, some `ioctl` commands are hardware specific and are processed by calling `dev_ioctl()`. Subsequently, depending on the hardware (STM32 or AT32), this function calls `fdcan_ioctl` or `at32can_ioctl`, as shown in the snippets that follow. ```c static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg) { FAR struct inode *inode = filep->f_inode; FAR struct can_dev_s *dev = inode->i_private; FAR struct can_reader_s *reader = filep->f_priv; ... flags = enter_critical_section(); /* Handle built-in ioctl commands */ switch (cmd) { ... /* Not a "built-in" ioctl command.. perhaps it is unique to this * lower-half, device driver. */ default: { ret = dev_ioctl(dev, cmd, arg); } break; } leave_critical_section(flags); return ret; } ``` There are a few instances where the user can trigger a kernel crash, caused by a division by zero on `CANIOC_SET_BITTIMING` command. On STM32 platforms (arch/arm/src/stm32/stm32_fdcan.c), while there is a `DEBUGASSERT` assertion for `bt->bt_baud`, the check does not cover value `0`. ```c static const struct can_ops_s g_fdcanops = { ... .co_ioctl = fdcan_ioctl, ... }; static int fdcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) { ... switch (cmd) { ... case CANIOC_SET_BITTIMING: { const struct canioc_bittiming_s *bt = (const struct canioc_bittiming_s *)arg; uint32_t nbrp; uint32_t ntseg1; uint32_t ntseg2; uint32_t nsjw; uint32_t ie; uint8_t state; DEBUGASSERT(bt != NULL); DEBUGASSERT(bt->bt_baud < STM32_FDCANCLK_FREQUENCY); // <- not valid DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16); /* Extract bit timing data */ ntseg1 = bt->bt_tseg1 - 1; ntseg2 = bt->bt_tseg2 - 1; nsjw = bt->bt_sjw - 1; nbrp = (uint32_t) ( ((float) STM32_FDCANCLK_FREQUENCY / ((float)(ntseg1 + ntseg2 + 3) * (float)bt->bt_baud)) - 1 ); // <- div by 0 ``` Similarly, another division by zero was found in Artery Technology AT32 (arch/arm/src/stm32/stm32_can.c) driver: ```c static const struct can_ops_s g_canops = { ... .co_ioctl = at32can_ioctl, ... }; static int at32can_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) { ... /* Handle the command */ switch (cmd) { ... case CANIOC_SET_BITTIMING: { const struct canioc_bittiming_s *bt = (const struct canioc_bittiming_s *)arg; ... uint32_t tmp; uint32_t regval; DEBUGASSERT(bt != NULL); DEBUGASSERT(bt->bt_baud < AT32_PCLK1_FREQUENCY); // <- not valid DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4); DEBUGASSERT(bt->bt_tseg1 > 0 && bt->bt_tseg1 <= 16); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 8); regval = at32can_getreg(priv, AT32_CAN_BTMG_OFFSET); /* Extract bit timing data tmp is in clocks per bit time */ tmp = AT32_PCLK1_FREQUENCY / bt->bt_baud; // <- div by 0 ``` Instances found are listed in the *Location* section below. They are not shown in detail to reduce the length of the issue. Ensure the attacker-controlled data is properly validated before use, to stop division by zero situations. For instance: ```c DEBUGASSERT(bt->bt_baud > 0 && bt->bt_baud < AT32_PCLK1_FREQUENCY); ``` * arch/arm/src/stm32/stm32_fdcan.c * arch/arm/src/stm32/stm32_can.c * arch/arm/src/sama5/sam_mcan.c * arch/arm/src/at32/at32_can.c * arch/arm/src/samv7/sam_mcan.c * arch/arm/src/stm32f0l0g0/stm32_fdcan.c * arch/arm/src/stm32f7/stm32_can.c * arch/arm/src/stm32h5/stm32_fdcan.c * arch/arm/src/stm32l4/stm32l4_can.c * arch/arm/src/tiva/common/tiva_can.c * drivers/can/mcp2515.c Signed-off-by: Catalin Visinescu --- arch/arm/src/at32/at32_can.c | 2 +- arch/arm/src/common/stm32/stm32_can_m3m4_v1.c | 3 ++- arch/arm/src/common/stm32/stm32_fdcan_m0_v1.c | 3 ++- arch/arm/src/common/stm32/stm32_fdcan_m3m4_v1.c | 3 ++- arch/arm/src/sama5/sam_mcan.c | 3 ++- arch/arm/src/samv7/sam_mcan.c | 3 ++- arch/arm/src/stm32f7/stm32_can.c | 3 ++- arch/arm/src/stm32h5/stm32_fdcan.c | 3 ++- arch/arm/src/stm32l4/stm32l4_can.c | 3 ++- arch/arm/src/tiva/common/tiva_can.c | 8 +++++--- drivers/can/mcp2515.c | 3 ++- 11 files changed, 24 insertions(+), 13 deletions(-) diff --git a/arch/arm/src/at32/at32_can.c b/arch/arm/src/at32/at32_can.c index 89b6dc43dab..1865f9cf97b 100644 --- a/arch/arm/src/at32/at32_can.c +++ b/arch/arm/src/at32/at32_can.c @@ -973,7 +973,7 @@ static int at32can_ioctl(struct can_dev_s *dev, int cmd, uint32_t regval; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < AT32_PCLK1_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && bt->bt_baud < AT32_PCLK1_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4); DEBUGASSERT(bt->bt_tseg1 > 0 && bt->bt_tseg1 <= 16); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 8); diff --git a/arch/arm/src/common/stm32/stm32_can_m3m4_v1.c b/arch/arm/src/common/stm32/stm32_can_m3m4_v1.c index 90959870c49..10fe28be2c8 100644 --- a/arch/arm/src/common/stm32/stm32_can_m3m4_v1.c +++ b/arch/arm/src/common/stm32/stm32_can_m3m4_v1.c @@ -971,7 +971,8 @@ static int stm32can_ioctl(struct can_dev_s *dev, int cmd, uint32_t regval; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < STM32_PCLK1_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < STM32_PCLK1_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4); DEBUGASSERT(bt->bt_tseg1 > 0 && bt->bt_tseg1 <= 16); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 8); diff --git a/arch/arm/src/common/stm32/stm32_fdcan_m0_v1.c b/arch/arm/src/common/stm32/stm32_fdcan_m0_v1.c index 8c72dd6b209..1cb8fb9a9a4 100644 --- a/arch/arm/src/common/stm32/stm32_fdcan_m0_v1.c +++ b/arch/arm/src/common/stm32/stm32_fdcan_m0_v1.c @@ -1832,7 +1832,8 @@ static int fdcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) uint8_t state; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < STM32_FDCANCLK_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < STM32_FDCANCLK_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16); diff --git a/arch/arm/src/common/stm32/stm32_fdcan_m3m4_v1.c b/arch/arm/src/common/stm32/stm32_fdcan_m3m4_v1.c index 4e23a563b1c..300545fefee 100644 --- a/arch/arm/src/common/stm32/stm32_fdcan_m3m4_v1.c +++ b/arch/arm/src/common/stm32/stm32_fdcan_m3m4_v1.c @@ -2136,7 +2136,8 @@ static int fdcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) uint8_t state; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < STM32_FDCANCLK_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < STM32_FDCANCLK_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16); diff --git a/arch/arm/src/sama5/sam_mcan.c b/arch/arm/src/sama5/sam_mcan.c index 7860c0a5afa..0b515fec719 100644 --- a/arch/arm/src/sama5/sam_mcan.c +++ b/arch/arm/src/sama5/sam_mcan.c @@ -2721,7 +2721,8 @@ static int mcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) uint8_t state; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < SAMA5_MCANCLK_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < SAMA5_MCANCLK_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16); diff --git a/arch/arm/src/samv7/sam_mcan.c b/arch/arm/src/samv7/sam_mcan.c index 847a25d31c5..164bba5d0a4 100644 --- a/arch/arm/src/samv7/sam_mcan.c +++ b/arch/arm/src/samv7/sam_mcan.c @@ -2789,7 +2789,8 @@ static int mcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) uint8_t state; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < SAMV7_MCANCLK_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < SAMV7_MCANCLK_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16); diff --git a/arch/arm/src/stm32f7/stm32_can.c b/arch/arm/src/stm32f7/stm32_can.c index 2d571c61b8d..2df5a048649 100644 --- a/arch/arm/src/stm32f7/stm32_can.c +++ b/arch/arm/src/stm32f7/stm32_can.c @@ -912,7 +912,8 @@ static int stm32can_ioctl(struct can_dev_s *dev, int cmd, uint32_t regval; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < STM32_PCLK1_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < STM32_PCLK1_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4); DEBUGASSERT(bt->bt_tseg1 > 0 && bt->bt_tseg1 <= 16); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 8); diff --git a/arch/arm/src/stm32h5/stm32_fdcan.c b/arch/arm/src/stm32h5/stm32_fdcan.c index 639f0e7e1d9..1be0ff65cbc 100644 --- a/arch/arm/src/stm32h5/stm32_fdcan.c +++ b/arch/arm/src/stm32h5/stm32_fdcan.c @@ -2007,7 +2007,8 @@ static int fdcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg) uint8_t state; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < STM32_FDCANCLK_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < STM32_FDCANCLK_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16); diff --git a/arch/arm/src/stm32l4/stm32l4_can.c b/arch/arm/src/stm32l4/stm32l4_can.c index 9d6231f4f1f..84225c4cc56 100644 --- a/arch/arm/src/stm32l4/stm32l4_can.c +++ b/arch/arm/src/stm32l4/stm32l4_can.c @@ -848,7 +848,8 @@ static int stm32l4can_ioctl(struct can_dev_s *dev, int cmd, uint32_t regval; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < STM32_PCLK1_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < STM32_PCLK1_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4); DEBUGASSERT(bt->bt_tseg1 > 0 && bt->bt_tseg1 <= 16); DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 8); diff --git a/arch/arm/src/tiva/common/tiva_can.c b/arch/arm/src/tiva/common/tiva_can.c index 57e5a96cec7..a28eb70f3e3 100644 --- a/arch/arm/src/tiva/common/tiva_can.c +++ b/arch/arm/src/tiva/common/tiva_can.c @@ -971,11 +971,13 @@ static int tivacan_ioctl(struct can_dev_s *dev, int cmd, timing.tseg1 = bt->bt_tseg1; timing.tseg2 = bt->bt_tseg2; timing.sjw = bt->bt_sjw; + DEBUGASSERT(timing.tseg1 <= 16 && timing.tseg1 >= 1); + DEBUGASSERT(timing.tseg2 <= 8 && timing.tseg2 >= 1); + DEBUGASSERT(timing.sjw <= 4 && timing.sjw >= 1); + DEBUGASSERT(bt->bt_baud > 0); + timing.prescaler = SYSCLK_FREQUENCY / (bt->bt_baud * (bt->bt_tseg1 + bt->bt_tseg2 + 1)); - DEBUGASSERT(timing.tseg1 <= 16 && timing.tseg1 >= 1); - DEBUGASSERT(timing.tseg2 <= 8 && timing.tseg1 >= 1); - DEBUGASSERT(timing.sjw <= 4 && timing.sjw >= 1); DEBUGASSERT(timing.prescaler <= 1024 && timing.prescaler >= 1); tivacan_bittiming_set(dev, &timing); diff --git a/drivers/can/mcp2515.c b/drivers/can/mcp2515.c index 3246da490d3..e31b49fbee3 100644 --- a/drivers/can/mcp2515.c +++ b/drivers/can/mcp2515.c @@ -1526,7 +1526,8 @@ static int mcp2515_ioctl(FAR struct can_dev_s *dev, int cmd, uint8_t regval; DEBUGASSERT(bt != NULL); - DEBUGASSERT(bt->bt_baud < MCP2515_CANCLK_FREQUENCY); + DEBUGASSERT(bt->bt_baud > 0 && + bt->bt_baud < MCP2515_CANCLK_FREQUENCY); DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4); DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 16); DEBUGASSERT(bt->bt_tseg2 > 1 && bt->bt_tseg2 <= 8);