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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Buffer overflow when parsing the 'microchip,calib-sensitivity' device tree property for devices with more than 8 channels.
- [Medium] Integer truncation silently drops signal guard configurations for channels 8-13.
- [Low] Out-of-bounds `GENMASK` generates an 11-bit mask for an 8-bit LED control register.
- [Low] Unnecessary 9th threshold register added to regmap defaults.
--

commit a843fc20f78c2766e479b33ffb3dcd6238d1f3f0
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 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 d45bb231d7a1f..e4da0e26c34a8 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c
[ ... ]
> @@ -98,6 +105,9 @@ static const struct reg_default cap11xx_reg_defaults[] = {
>  	{ CAP11XX_REG_SENSOR_THRESH(3),		0x40 },
>  	{ CAP11XX_REG_SENSOR_THRESH(4),		0x40 },
>  	{ CAP11XX_REG_SENSOR_THRESH(5),		0x40 },
> +	{ CAP11XX_REG_SENSOR_THRESH(6),		0x40 },
> +	{ CAP11XX_REG_SENSOR_THRESH(7),		0x40 },
> +	{ CAP11XX_REG_SENSOR_THRESH(8),		0x40 },

[Severity: Low]
Does this code inadvertently add a 9th threshold register?

Since the indexing is 0-based, an 8-threshold device should only need indices
0 through 7.

Register index 8 appears to map to an unused register.

>  	{ CAP11XX_REG_CONFIG2,			0x40 },
>  };
[ ... ]
> @@ -394,6 +442,14 @@ static int cap11xx_init_leds(struct device *dev,
>  	if (error)
>  		return error;
>  
> +	if (num_leds > 8) {
> +		error = regmap_update_bits(priv->regmap,
> +					   priv->model->led_output_control_reg_base + 1,
> +					   GENMASK(num_leds - 8 - 1, 0), 0);

[Severity: Low]
Can this result in an out-of-bounds mask generation when num_leds > 8?

In cap11xx_init_leds() just prior to this addition, the code calls:

    error = regmap_update_bits(priv->regmap,
                   priv->model->led_output_control_reg_base,
                   GENMASK(num_leds - 1, 0), 0);

When num_leds is 11 for the CAP1114 model, this generates an 11-bit mask
to update an 8-bit register.

While the regmap core safely masks this, should the mask be clamped to
min(num_leds, 8) to accurately reflect the hardware register's capacity?

> +		if (error)
> +			return error;
> +	}
[ ... ]
> @@ -572,6 +628,14 @@ static const struct cap11xx_hw_model cap1106_model = {
>  	.has_repeat_en = true,
>  };
>  
> +static const struct cap11xx_hw_model cap1114_model = {
> +	.product_id = 0x3a,
> +	.num_channels = 14, .num_leds = 11, .num_sensor_thresholds = 8,

[Severity: High]
Does this code overflow the calib_sensitivities array during initialization?

In struct cap11xx_priv, the array is rigidly sized to 8 elements:

    u32 calib_sensitivities[8];
    u32 keycodes[];

During cap11xx_init_keys(), when parsing "microchip,calib-sensitivity", the
driver reads priv->model->num_channels elements:

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

With num_channels set to 14, won't this write 14 elements into an 8-element
array, clobbering the adjacent keycodes flexible array member and potentially
causing memory corruption or a kernel panic?


[Severity: Medium]
Will this configuration lead to integer truncation when parsing the
"microchip,signal-guard" property?

In struct cap11xx_priv, the mask is defined as an 8-bit value:

    u8 signal_guard_inputs_mask;

During cap11xx_init_keys(), a loop iterates up to num_channels (14) and shifts
the bits:

    priv->signal_guard_inputs_mask |= 0x01 << i;

For channels 8 to 13, won't this bitwise shift exceed the 8-bit variable width
and silently truncate to 0?

Even if the CAP1114 doesn't support signal guards, this drops the bitmask for
the upper channels, bypassing the capability check and failing to emit the
intended 'unsupported' warning.

> +	.led_output_control_reg_base = CAP1114_REG_LED_OUTPUT_CONTROL1,
> +	.sensor_input_reg_base = CAP1114_REG_BUTTON_STATUS1,
> +	.has_grouped_sensors = true,
> +};

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