-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
i2c_slave.c: fix buffer overrun on s_i2c_handle_complete() (IDFGH-13973) #14804
Conversation
Fixing a buffer overrun of i2c_slave->data_buf. The i2c_ll_read_rxfifo function was using t->rcv_fifo_cnf (the I2C slave reading code's buffer size) as the limit for how many bytes on write on i2c_slave->data_buf. This buffer size for i2c_slave->data_buf is generally smaller than the buffer that the I2C slave reading code has.
|
👋 Hello danielcolchete, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Thanks for the contribution. We notice that the api in i2c_slave.c has several issues, and lack ease of use. So we are coming with new i2c slave driver in i2c_slave_v2.c (will be online both master and v5.4). Meanwhile, we will slow down the maintainance of |
@mythbuster5 how does "slow down the maintenance of i2c_slave.c" translates into not accepting memory corruption bugfixes? |
Fixing a buffer overrun of i2c_slave->data_buf.
The i2c_ll_read_rxfifo function was using t->rcv_fifo_cnf (the I2C slave reading code's buffer size) as the limit for how many bytes on write on i2c_slave->data_buf.
This buffer size for i2c_slave->data_buf is generally smaller than the buffer that the I2C slave reading code has.
Description
Issue #14803 explains the problem, but TL/DR the i2c_slave_receive() workflow causes memory corruption due to a buffer overrun. And then it generally gets to a
Core 0 panic'ed (StoreProhibited). Exception was unhandled.
error on runtime.Related
Fixes #14803
Documentation used for my own test case: https://docs.espressif.com/projects/esp-idf/en/v5.3.1/esp32/api-reference/peripherals/i2c.html
Testing
Tested locally on a couple of ESP32-DevKitC-V4 boards.
Checklist
Before submitting a Pull Request, please ensure the following: