Re: [PATCH 8/8] Input: cap11xx - add support for CAP1114

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Buffer overflow when reading the 'microchip,calib-sensitivity' device tree property for devices with >8 channels.
- [High] Failure to mask `STATUS1` before ORing `STATUS2` can cause phantom key presses for channels 7 and 8.
- [Low] Bitmask calculation for 'microchip,signal-guard' truncates settings for channels 8 and above.
--

commit 8885b1d0f98cb8d5ca53ba5fd9a458743481d5be
Author: Jun Yan <jerrysteve1101@gmail.com>

Input: cap11xx - add support for CAP1114

CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support. The CAP1114 uses two separate control registers
for LED output management and requires two button status registers for touch
input state reporting.

> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 408880cf65167..2aba3b508947d 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c

[ ... ]

> @@ -311,6 +337,20 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
>  	if (ret < 0)
>  		goto out;
>  
> +	if (priv->model->num_channels > 8) {
> +		unsigned int status2;
> +
> +		ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base + 1, &status2);
> +		if (ret < 0)
> +			goto out;
> +
> +		/*
> +		 * CAP1114 STATUS1 register only contains data for the first 6 channels.
> +		 * the remaining channels is stored in STATUS2.
> +		 */
> +		status |= FIELD_PREP(GENMASK(13, 6), status2);

[Severity: High]
If the hardware sets reserved bits 6 or 7 in the first status register to 1,
will they be preserved and incorrectly reported as touch events for channels
7 and 8?

Since the status variable (which holds STATUS1) is not masked before this OR
operation, any reserved bits set in STATUS1 could overlap with the shifted
status2 values.

Should the status variable be masked to clear bits 6 and 7 before ORing in
status2?

> +	}
> +
>  	for (i = 0; i < priv->idev->keycodemax; i++)

[ ... ]

> @@ -572,6 +625,14 @@ static const struct cap11xx_hw_model cap1106_model = {
>  	.has_repeat_en = true,
>  };
>  
> +static const struct cap11xx_hw_model cap1114_model = {
> +	.product_id = 0x3a,
> +	.led_output_control_reg_base = CAP1114_REG_LED_OUTPUT_CONTROL1,
> +	.sensor_input_reg_base = CAP1114_REG_BUTTON_STATUS1,
> +	.num_channels = 14, .num_leds = 11, .num_sensor_thresholds = 8,

[Severity: High]
Does configuring num_channels to 14 cause a buffer overflow when parsing the
device tree?

In struct cap11xx_priv, calib_sensitivities is defined as a fixed 8-element
array:

struct cap11xx_priv {
        ...
        u32 calib_sensitivities[8];
        u32 keycodes[];
};

When cap11xx_init_keys() parses the microchip,calib-sensitivity property, it
requests num_channels elements:

of_property_read_u32_array(node, "microchip,calib-sensitivity",
                           priv->calib_sensitivities,
                           priv->model->num_channels)

If the device tree provides 14 elements, won't this write 56 bytes into the
32-byte array, overflowing into the adjacent keycodes flexible array?

[Severity: Low]
Additionally, does setting num_channels to 14 cause truncation in the
microchip,signal-guard bitmask logic?

In cap11xx_init_keys(), priv->signal_guard_inputs_mask is typed as a u8, but
the initialization loop runs up to num_channels:

if (u32_val)
        priv->signal_guard_inputs_mask |= 0x01 << i;

For i >= 8, does the shifted bit exceed the 8-bit width and get silently
truncated to 0 during the assignment?

> +	.has_grouped_sensors = true,
> +};
> +

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