Re: [PATCH v13 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] input: keyboard: u8 overflow and page mixup in asus_ec_keys_process_key()
--

commit ba1df1a44a7f539aae799312892bb55d53afd18c
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>

input: keyboard: Add driver for ASUS Transformer dock multimedia keys

This commit adds an input driver for the ASUS Transformer Mobile Dock
multimedia top button row, defaulting to F1-F12 function keys and allowing
switching to multimedia keys with ScreenLock + AltGr.

> diff --git a/drivers/input/keyboard/asus-transformer-ec-keys.c b/drivers/input/keyboard/asus-transformer-ec-keys.c
> new file mode 100644
> index 0000000000000..5a8fb86a97555
> --- /dev/null
> +++ b/drivers/input/keyboard/asus-transformer-ec-keys.c
> @@ -0,0 +1,216 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/array_size.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/mfd/asus-transformer-ec.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#define ASUSEC_EXT_KEY_CODES		0x20
> +
> +struct asus_ec_keys_data {
> +	struct notifier_block nb;
> +	struct asusec_core *ec;
> +	struct input_dev *xidev;
> +	unsigned short keymap[ASUSEC_EXT_KEY_CODES * 2];
> +	bool special_key_mode;
> +};

[ ... ]

> +static int asus_ec_keys_process_key(struct input_dev *dev, u8 code)
> +{
> +	struct asus_ec_keys_data *priv = dev_get_drvdata(dev->dev.parent);
> +	unsigned int key = 0;
> +
> +	if (code == 0)
> +		return NOTIFY_DONE;
> +
> +	if (priv->ec->right_alt_pressed) {
> +		priv->ec->special_combo_triggered = true;
> +
> +		/* Flip special key mode state when pressing SCREEN LOCK + R ALT */
> +		if (code == 1) {
> +			priv->special_key_mode = !priv->special_key_mode;
> +			return NOTIFY_OK;
> +		}
> +	}
> +
> +	/*
> +	 * Relocate code to second "page" if pressed state XOR's mode state
> +	 * This way special key will invert the current mode
> +	 */
> +	if (priv->special_key_mode ^ priv->ec->right_alt_pressed)
> +		code += ASUSEC_EXT_KEY_CODES;
> +
> +	if (code < dev->keycodemax) {
> +		unsigned short *map = dev->keycode;
> +
> +		key = map[code];
> +	}
> +
> +	if (!key)
> +		key = KEY_UNKNOWN;
> +
> +	asus_ec_keys_report_key(dev, code, key, 1);
> +	asus_ec_keys_report_key(dev, code, key, 0);
> +
> +	return NOTIFY_OK;
> +}

[Severity: Medium]
Should asus_ec_keys_process_key() validate that code < ASUSEC_EXT_KEY_CODES
before setting priv->ec->special_combo_triggered and offsetting code by
ASUSEC_EXT_KEY_CODES?

In asus_ec_keys_setup_keymap(), dev->keycodemax is set to
ARRAY_SIZE(priv->keymap) (0x40), spanning both keymap pages:

drivers/input/keyboard/asus-transformer-ec-keys.c:asus_ec_keys_setup_keymap() {
	...
	dev->keycode = priv->keymap;
	dev->keycodesize = sizeof(*priv->keymap);
	dev->keycodemax = ARRAY_SIZE(priv->keymap);
	...
}

When asus_ec_keys_notify() passes data[2] to asus_ec_keys_process_key() with a
raw code >= ASUSEC_EXT_KEY_CODES (0x20), a few issues can occur:

1. If priv->ec->right_alt_pressed is true, priv->ec->special_combo_triggered is
   set to true even for out-of-range codes.

2. When (priv->special_key_mode ^ priv->ec->right_alt_pressed) is false, a raw
   code in 0x20..0x3f passes code < dev->keycodemax (0x40) and indexes into
   the second page (multimedia keys) in map[code] instead of reporting
   KEY_UNKNOWN.

3. When (priv->special_key_mode ^ priv->ec->right_alt_pressed) is true, adding
   ASUSEC_EXT_KEY_CODES (0x20) to the 8-bit u8 code overflows for raw codes in
   0xe0..0xff, wrapping code into 0x00..0x1f (the first page of map[]) and
   reporting the wrapped value via MSC_SCAN instead of KEY_UNKNOWN. For raw
   codes in 0x20..0xdf, MSC_SCAN is also reported with code + 0x20 rather than
   the raw code.

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