[PATCH] Input: matrix-keymap - reject keycodes above KEY_MAX

[PATCH] Input: matrix-keymap - reject keycodes above KEY_MAX

[Linmao Li]

matrix_keypad_map_key() validates the row and column of each entry before
using them to update the keymap. It does not validate the keycode before
using it as a bit number in input_dev->keybit, which only has KEY_CNT bits.
A malformed firmware or platform keymap can therefore write past the
bitmap.

Reject keycodes above KEY_MAX before updating the keymap or capability
bitmap.

Fixes: 77a53fd21870 ("Input: matrix-keypad - add function to build device keymap")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/input/matrix-keymap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
index 3bea3575a0a9..e4227e96aa4e 100644
--- a/drivers/input/matrix-keymap.c
+++ b/drivers/input/matrix-keymap.c
@@ -35,6 +35,13 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
 		return false;
 	}
 
+	if (code > KEY_MAX) {
+		dev_err(input_dev->dev.parent,
+			"%s: invalid keycode %u in keymap entry 0x%x\n",
+			__func__, code, key);
+		return false;
+	}
+
 	keymap[MATRIX_SCAN_CODE(row, col, row_shift)] = code;
 	__set_bit(code, input_dev->keybit);
 

base-commit: 944a035ecca915ae947905dcfb03f2b9dc6d032c
-- 
2.25.1

Re: [PATCH] Input: matrix-keymap - reject keycodes above KEY_MAX

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing bounds checks on matrix dimensions causes an integer overflow when calculating max_keys, leading to an undersized heap allocation and a subsequent out-of-bounds write.
--

commit 821b9503dcfef31a275bc1fec44950b4eda55e94
Author: Linmao Li <lilinmao@kylinos.cn>

Input: matrix-keymap - reject keycodes above KEY_MAX

This commit adds bounds checking for keycodes in matrix_keypad_map_key() to
ensure they do not exceed KEY_MAX. This prevents a malformed firmware or
platform keymap from writing past the bounds of the capability bitmap.

> diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
> index 3bea3575a0a94..e4227e96aa4e7 100644
> --- a/drivers/input/matrix-keymap.c
> +++ b/drivers/input/matrix-keymap.c
> @@ -35,6 +35,13 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
>  		return false;
>  	}
>  
> +	if (code > KEY_MAX) {
> +		dev_err(input_dev->dev.parent,
> +			"%s: invalid keycode %u in keymap entry 0x%x\n",
> +			__func__, code, key);
> +		return false;
> +	}
> +
>  	keymap[MATRIX_SCAN_CODE(row, col, row_shift)] = code;

[Severity: High]
This is a pre-existing issue, but since this patch aims to mitigate malformed
firmware and keymaps causing out-of-bounds writes, does the bounds check
earlier in this function fully protect against integer overflow?

If a maliciously crafted Device Tree provides a large value for
keypad,num-rows (e.g., 0x80000001), it is read without bounds checking. When
matrix_keypad_build_keymap() computes max_keys:

drivers/input/matrix-keymap.c:matrix_keypad_build_keymap() {
    ...
    size_t max_keys = rows << row_shift;
    ...
    keymap = devm_kcalloc(&input_dev->dev, max_keys, sizeof(*keymap),
                          GFP_KERNEL);
}

The 32-bit shift overflows, resulting in a tiny allocation for keymap.

Then in matrix_keypad_map_key(), the earlier bounds check:

    if (row >= rows || col >= cols) {
        return false;
    }

compares the extracted 8-bit row against the large unshifted rows value
(0x80000001), completely bypassing the safety check.

This allows MATRIX_SCAN_CODE to calculate an index far outside the tiny
buffer, causing an out-of-bounds heap write when assigning the keycode here.
Should rows and cols be validated against a maximum sane value before the
shift to prevent this overflow?

>  	__set_bit(code, input_dev->keybit);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909100739.3107556-1-lilinmao@kylinos.cn?part=1