[Coverity CID: 520269] Unintentional integer overflow in drivers/sensor/bosch/bma4xx/bma4xx_emul.c

[zephyrbot]

Static code scan issues found in file:

https://github.com/zephyrproject-rtos/zephyr/tree/265cfb45a818f39cf03c3527b5d1b21027d3cac1/drivers/sensor/bosch/bma4xx/bma4xx_emul.c

Category: Integer handling issues
Function: bma4xx_emul_set_accel_data
Component: Drivers
CID: 520269

Details:

zephyr/drivers/sensor/bosch/bma4xx/bma4xx_emul.c

Line 206 in 265cfb4

int64_t accel_range = (2 << data->regs[BMA4XX_REG_ACCEL_RANGE]);

Please fix or provide comments in coverity using the link:

https://scan9.scan.coverity.com/#/project-view/29271/12996?selectedIssue=520269

For more information about the violation, check the Coverity Reference. (CWE-190)

Note: This issue was created automatically. Priority was set based on classification
of the file affected and the impact field in coverity. Assignees were set using the MAINTAINERS file.

[ldomaigne]

The data->regs[BMA4XX_REG_ACCEL_RANGE] is an uint8_t, but only the first two bits is used per BMA456 spec (chap 5, register 0x41 ACC_RANGE). This fact is correctly implemented in bma4xx_emul_write_byte and bma4xx_emul_backend_get_sample_range. Therefore, we can't have an integer overflow. From my understanding, it's a false positive.

We could either document it in Coverity, or change the line as suggested below with the hope that coverity can infer that we have at a shift of 3 max:

 int64_t accel_range = (2 << (data->regs[BMA4XX_REG_ACCEL_RANGE] & GENMASK(1,0))  ); 

@MaureenHelm : what is your preferred approach? Update: it's probably easier than first thought: cast 2 (int by default) to uint64_t... So, no question left for now! 😉

[ljd42]

@kartben: good first issue!

No overflow is current happening in code, improvement suggested by Coverity is straightforward to implement. It's a good first issue to get familiar with Zephyr's contribution workflow.

---

Here's the improvement suggested by Coverity:

zephyr/drivers/sensor/bosch/bma4xx/bma4xx_emul.c

Line 206 in 265cfb4

int64_t accel_range = (2 << data->regs[BMA4XX_REG_ACCEL_RANGE]);

CID 520269: (1 of 1): Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression 2 << data->regs[65] with type int (32 bits, signed) is evaluated
using 32-bit arithmetic, and then used in a context that expects an expression of type int64_t (64 bits, signed).

To avoid overflow, cast 2 to type int64_t.