Skip to content

Commit

Permalink
drivers: i2c: i2c_dw: Fixed integer overflow in i2c_dw_data_ask().
Browse files Browse the repository at this point in the history
The controller can implement a reception FIFO as deep as 256 bytes.
However, the computation made by the driver code to determine how many
bytes can be asked is stored in a signed 8-bit variable called rx_empty.

If the reception FIFO depth is greater or equal to 128 bytes and the FIFO
is currently empty, the rx_empty value will be 128 (or more), which
stands for a negative value as the variable is signed.

Thus, the later code checking if the FIFO is full will run while it should
not and exit from the i2c_dw_data_ask() function too early.

This hangs the controller in an infinite loop of interrupt storm because
the interrupt flags are never cleared.

Storing the rx_empty empty on a signed 32-bit variable instead of a 8-bit
one solves the issue and is compliant with the controller hardware
specifications of a maximum FIFO depth of 256 bytes.

It has been agreed with upstream maintainers to change the type of the
variables tx_empty, rx_empty, cnt, rx_buffer_depth and tx_buffer_depth to
plain int because it is most effectively handled by the CPUs. Using 8-bit
or 16-bit variables had no meaning here.

Signed-off-by: Adrien Ricciardi <[email protected]>
(cherry picked from commit 4824e40)
  • Loading branch information
RICCIARDI-Adrien authored and github-actions[bot] committed Jan 9, 2024
1 parent 32748c6 commit 0978286
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions drivers/i2c/i2c_dw.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ static inline void i2c_dw_data_ask(const struct device *dev)
{
struct i2c_dw_dev_config * const dw = dev->data;
uint32_t data;
uint8_t tx_empty;
int8_t rx_empty;
uint8_t cnt;
uint8_t rx_buffer_depth, tx_buffer_depth;
int tx_empty;
int rx_empty;
int cnt;
int rx_buffer_depth, tx_buffer_depth;
union ic_comp_param_1_register ic_comp_param_1;
uint32_t reg_base = get_regs(dev);

Expand Down

0 comments on commit 0978286

Please sign in to comment.