Re: [PATCH] Input: tca8418_keypad - enable overflow mode per datasheet (SCPS215G)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver does not sufficiently validate the hardware keycode, allowing a malformed I2C read to cause an out-of-bounds array access.
- [High] The keypad FIFO read loop lacks an upper iteration bound, risking an infinite loop if the I2C bus fails silently.
--

commit c17e8cb1a67b8fa0dfa772dc4d63c35917e63ccb
Author: Zhian Liang <liangzhan5dev@gmail.com>

Input: tca8418_keypad - enable overflow mode per datasheet (SCPS215G)

This commit sets the OVR_FLOW_M bit alongside OVR_FLOW_IEN in the
configuration register. This ensures the overflow interrupt is properly
generated, preventing FIFO overflow events from being silently lost.

> diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
> index 68c0afafee7bf..b124e576feca4 100644
> --- a/drivers/input/keyboard/tca8418_keypad.c
> +++ b/drivers/input/keyboard/tca8418_keypad.c
> @@ -254,7 +254,7 @@ static int tca8418_configure(struct tca8418_keypad *keypad_data,
>  		return error;
>  
>  	error = tca8418_write_byte(keypad_data, REG_CFG,
> -				CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_KE_IEN);
> +				CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_OVR_FLOW_M | CFG_KE_IEN);
>  
>  	return error;
>  }

While reviewing this, I noticed a couple of potential issues in the same
file.

drivers/input/keyboard/tca8418_keypad.c:tca8418_read_keypad() {
	[ ... ]
	do {
		error = tca8418_read_byte(keypad_data, REG_KEY_EVENT_A, &reg);
		if (error < 0) {
			dev_err(&keypad_data->client->dev,
				"unable to read REG_KEY_EVENT_A\n");
			break;
		}

		/* Assume that key code 0 signifies empty FIFO */
		if (reg <= 0)
			break;
	[ ... ]

[Severity: High]
This is a pre-existing issue, but does this loop risk running infinitely if
the I2C bus fails silently?

If the I2C bus gets stuck high, it may return 0xFF. Because reg is an
unsigned u8, the condition reg <= 0 evaluates to false for 0xFF (255).

The loop would never terminate, stalling the threaded IRQ handler
indefinitely.

	[ ... ]
		row = code / TCA8418_MAX_COLS;
		col = code % TCA8418_MAX_COLS;

		row = (col) ? row : row - 1;
		col = (col) ? col - 1 : TCA8418_MAX_COLS - 1;

		code = MATRIX_SCAN_CODE(row, col, keypad_data->row_shift);
		input_event(input, EV_MSC, MSC_SCAN, code);
		input_report_key(input, keymap[code], state);
	[ ... ]
}

[Severity: High]
This isn't a bug introduced by this patch, but does the driver sufficiently
validate the hardware keycode to prevent an out-of-bounds array access?

If the I2C bus returns 0xFF, code is calculated as 127. The index
calculations yield row = 12 and col = 6, resulting in code = 198.

Since the keymap array allocated for this device has a maximum of 128
elements, this results in an out-of-bounds memory read on keymap[198].

Similarly, a malformed value of 0x80 results in row = -1, col = 9, and
code = 249, causing the same out-of-bounds access.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529013900.43854-1-liangzhan5dev@gmail.com?part=1