mirror of
https://github.com/apache/nuttx.git
synced 2026-08-01 20:28:58 +00:00
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 <catalin_visinescu@yahoo.com>
This commit is contained in:
parent
fd4e34be6d
commit
140ad8b305
11 changed files with 24 additions and 13 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue