This is an automated email from the ASF dual-hosted git repository.
acassis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new b82ceb62a7a arch/arm/src/at32/at32_can: Division by Zero in CAN Bit
Timing Commands
b82ceb62a7a is described below
commit b82ceb62a7a45ab46ce4201fd5ca4b77caad16f6
Author: Catalin Visinescu <[email protected]>
AuthorDate: Mon Jun 15 00:35:53 2026 -0400
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 <[email protected]>
---
arch/arm/src/at32/at32_can.c | 2 +-
arch/arm/src/sama5/sam_mcan.c | 3 ++-
arch/arm/src/samv7/sam_mcan.c | 3 ++-
arch/arm/src/stm32/stm32_can.c | 3 ++-
arch/arm/src/stm32/stm32_fdcan.c | 3 ++-
arch/arm/src/stm32f0l0g0/stm32_fdcan.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/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/stm32/stm32_can.c b/arch/arm/src/stm32/stm32_can.c
index d0e8373aa36..35bc9e71781 100644
--- a/arch/arm/src/stm32/stm32_can.c
+++ b/arch/arm/src/stm32/stm32_can.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/stm32/stm32_fdcan.c b/arch/arm/src/stm32/stm32_fdcan.c
index 519b77626e9..2e2574217eb 100644
--- a/arch/arm/src/stm32/stm32_fdcan.c
+++ b/arch/arm/src/stm32/stm32_fdcan.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/stm32f0l0g0/stm32_fdcan.c
b/arch/arm/src/stm32f0l0g0/stm32_fdcan.c
index 02d2c0f773d..d32259614db 100644
--- a/arch/arm/src/stm32f0l0g0/stm32_fdcan.c
+++ b/arch/arm/src/stm32f0l0g0/stm32_fdcan.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/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;
- 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.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.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);
