Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Count overflow in Arduino/I2Cdev::readBytes and I2Cdev::readWords #751

Open
nekomona opened this issue May 28, 2023 · 1 comment
Open

Count overflow in Arduino/I2Cdev::readBytes and I2Cdev::readWords #751

nekomona opened this issue May 28, 2023 · 1 comment

Comments

@nekomona
Copy link
Contributor

The data type of count in these two functions are incorrectly being int8_t, while length is uint8_t. This will cause an overflow when transmitting data with length > 128 and corrupt the data before buffer.

*/
int8_t I2Cdev::readBytes(uint8_t devAddr, uint8_t regAddr, uint8_t length, uint8_t *data, uint16_t timeout, void *wireObj) {
#ifdef I2CDEV_SERIAL_DEBUG
Serial.print("I2C (0x");
Serial.print(devAddr, HEX);
Serial.print(") reading ");
Serial.print(length, DEC);
Serial.print(" bytes from 0x");
Serial.print(regAddr, HEX);
Serial.print("...");
#endif
int8_t count = 0;
uint32_t t1 = millis();

More occurrences have been found in #750 .

Below is an overflow captured when reading 168 bytes from a MPU6050 FIFO, which caused function frame corruption and crashed the program.
image

@nekomona
Copy link
Contributor Author

It seems that the sign of count is used to report failure, which need to be taken into consideration:
Whether should it limit length within 127 bytes, using 0~254 as normal count and 255 as failure, or extend the data width.

Currently it's the second one with implicit type conversion. Code detecting failure by comparing with -1 still works, yet if it only compares with 0, reads with length > 127 will be misinterpreted as failed even if the result is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant