Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions components/esp_driver_i2c/i2c_master.c
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,51 @@ esp_err_t i2c_master_transmit_receive(i2c_master_dev_handle_t i2c_dev, const uin
return ret;
}

esp_err_t i2c_master_multi_buffer_transmit_receive(i2c_master_dev_handle_t i2c_dev, i2c_master_transmit_multi_buffer_info_t *buffer_info_array, size_t array_size, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms)
{
ESP_RETURN_ON_FALSE(i2c_dev != NULL, ESP_ERR_INVALID_ARG, TAG, "i2c handle not initialized");
ESP_RETURN_ON_FALSE(array_size <= (SOC_I2C_CMD_REG_NUM - 2 - 3), ESP_ERR_INVALID_ARG, TAG, "i2c command list cannot contain so many commands");
ESP_RETURN_ON_FALSE(buffer_info_array != NULL, ESP_ERR_INVALID_ARG, TAG, "buffer info array is empty");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing validation causes integer underflow when read_size is zero

The new function i2c_master_multi_buffer_transmit_receive() lacks validation for read_buffer and read_size, unlike the similar functions i2c_master_transmit_receive() and i2c_master_receive() which check (read_buffer != NULL) && (read_size > 0). When read_size is zero, the expression read_size - 1 causes unsigned integer underflow to SIZE_MAX, and read_buffer + read_size - 1 creates an out-of-bounds pointer.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear AI, I copied the function basically from the one above. I have no interest in fixing this issue when it was already present (and accepted?) previously.


esp_err_t ret = ESP_OK;
size_t op_index = 0;
i2c_operation_t i2c_ops[SOC_I2C_CMD_REG_NUM] = {};
i2c_ops[op_index++].hw_cmd.op_code = I2C_LL_CMD_RESTART;
int i;
for (i = 0; i < array_size; i++) {
if (buffer_info_array[i].buffer_size == 0) {
continue;
}
i2c_ops[op_index].hw_cmd.ack_en = i2c_dev->ack_check_disable ? false : true;
i2c_ops[op_index].hw_cmd.op_code = I2C_LL_CMD_WRITE;
i2c_ops[op_index].data = (uint8_t*)buffer_info_array[i].write_buffer;
i2c_ops[op_index].total_bytes = buffer_info_array[i].buffer_size;
i2c_ops[op_index].bytes_used = 0;
op_index++;
}

i2c_ops[op_index++].hw_cmd.op_code = I2C_LL_CMD_RESTART;

i2c_ops[op_index].hw_cmd.op_code = I2C_LL_CMD_READ;
i2c_ops[op_index].hw_cmd.ack_val = I2C_ACK_VAL;
i2c_ops[op_index].data = read_buffer;
i2c_ops[op_index++].total_bytes = read_size - 1;

i2c_ops[op_index].hw_cmd.op_code = I2C_LL_CMD_READ;
i2c_ops[op_index].hw_cmd.ack_val = I2C_NACK_VAL;
i2c_ops[op_index].data = (read_buffer + read_size - 1);
i2c_ops[op_index++].total_bytes = 1;

i2c_ops[op_index++].hw_cmd.op_code = I2C_LL_CMD_STOP;

if (i2c_dev->master_bus->async_trans == false) {
ret = s_i2c_synchronous_transaction(i2c_dev, i2c_ops, op_index, xfer_timeout_ms);
} else {
ret = s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, op_index, xfer_timeout_ms);
}
return ret;
}

esp_err_t i2c_master_receive(i2c_master_dev_handle_t i2c_dev, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms)
{
ESP_RETURN_ON_FALSE(i2c_dev != NULL, ESP_ERR_INVALID_ARG, TAG, "i2c handle not initialized");
Expand Down
23 changes: 23 additions & 0 deletions components/esp_driver_i2c/include/driver/i2c_master.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,29 @@ esp_err_t i2c_master_multi_buffer_transmit(i2c_master_dev_handle_t i2c_dev, i2c_
*/
esp_err_t i2c_master_transmit_receive(i2c_master_dev_handle_t i2c_dev, const uint8_t *write_buffer, size_t write_size, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms);

/**
* @brief Perform a multi-write-read transaction on the I2C bus.
* This function transmits multiple buffers of data and then reads.
* The transaction will be undergoing until it finishes or it reaches
* the timeout provided.
*
* @note If a callback was registered with `i2c_master_register_event_callbacks`, the transaction will be asynchronous, and thus, this function will return directly, without blocking.
* You will get finish information from callback. Besides, data buffer should always be completely prepared when callback is registered, otherwise, the data will get corrupt.
*
* @param[in] i2c_dev I2C master device handle that created by `i2c_master_bus_add_device`.
* @param[in] buffer_info_array Pointer to buffer information array.
* @param[in] array_size size of buffer information array.
* @param[out] read_buffer Data bytes received from i2c bus.
* @param[in] read_size Size, in bytes, of the read buffer.
* @param[in] xfer_timeout_ms Wait timeout, in ms. Note: -1 means wait forever.
* @return
* - ESP_OK: I2C master transmit-receive success.
* - ESP_ERR_INVALID_RESPONSE: I2C master transmit-receive receives NACK.
* - ESP_ERR_INVALID_ARG: I2C master transmit parameter invalid.
* - ESP_ERR_TIMEOUT: Operation timeout(larger than xfer_timeout_ms) because the bus is busy or hardware crash.
*/
esp_err_t i2c_master_multi_buffer_transmit_receive(i2c_master_dev_handle_t i2c_dev, i2c_master_transmit_multi_buffer_info_t *buffer_info_array, size_t array_size, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms);

/**
* @brief Perform a read transaction on the I2C bus.
* The transaction will be undergoing until it finishes or it reaches
Expand Down