Re: [PATCH v2 6/9] Input: cap11xx - add reset gpio support

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Unintended glitch on the reset line during probe due to GPIOD_OUT_LOW initialization.

Pre-existing issues:
- [High] Unvalidated keycodes from Device Tree cause an out-of-bounds heap write.
--

commit 5befbfbb7019f474fe2d2d2e0e3da23976b75ada
Author: Jun Yan <jerrysteve1101@gmail.com>

Input: cap11xx - add reset gpio support

Some CAP11xx devices (CAP1126/CAP1188) have a dedicated RESET pin. Add
hardware reset operation to improve device reliability and ensure proper
initialization on probe.

> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 686174722204c..75746a8a22332 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c

[ ... ]

> @@ -452,6 +457,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>  	if (IS_ERR(priv->regmap))
>  		return PTR_ERR(priv->regmap);
>  
> +	priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
> +				     "Failed to get 'reset' GPIO\n");
> +	if (priv->reset_gpio) {
> +		gpiod_set_value_cansleep(priv->reset_gpio, 1);

[Severity: Medium]
Does initializing the reset GPIO with GPIOD_OUT_LOW cause an unintended
glitch on the reset line?

When devm_gpiod_get_optional() is called with GPIOD_OUT_LOW, the GPIO line
is immediately driven to its logical low (inactive) state. If the hardware
or bootloader previously left the device in a reset state, this prematurely
pulls it out of reset.

The driver then immediately calls gpiod_set_value_cansleep() to assert
the reset. This sequence creates an extremely short, unintended
de-assert/assert glitch on the reset pin, which could violate hardware
timing requirements.

Would it be safer to request the reset GPIO with GPIOD_OUT_HIGH (or
GPIOD_ASIS) to guarantee it starts in the logically asserted state before
we hold it for the required minimum duration?

[Severity: High]
This is a pre-existing issue, but does this code overflow the keybit array
if a device tree provides unvalidated keycodes?

Later in cap11xx_i2c_probe(), the driver loops over the keycodes read from
the device tree:

for (i = 0; i < cap->num_channels; i++)
        __set_bit(priv->keycodes[i], priv->idev->keybit);

Since __set_bit() does not perform bounds checking, if a device tree
specifies a keycode value greater than or equal to KEY_MAX, this will
write outside the bounds of the keybit array in the input_dev structure,
clobbering adjacent memory in the kernel heap.

Should we add a check against KEY_MAX before setting the bit?

> +		usleep_range(CAP11XX_T_RST_FILT_MIN_US, CAP11XX_T_RST_FILT_MIN_US * 2);
> +		gpiod_set_value_cansleep(priv->reset_gpio, 0);
> +		msleep(CAP11XX_T_RST_ON_MIN_MS);
> +	}
> +
>  	error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);

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