Re: [PATCH v4 07/10] Input: cap11xx - refactor code for better CAP1114 support.

[sashiko-bot@kernel.org]

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

New issues:
- [High] Bitmask truncation in `cap11xx_led_set` causing silent failure for LEDs >= 8.
- [High] Regmap cache staleness due to missing volatile register definition for the dynamic `sensor_input_reg_base`.

Pre-existing issues:
- [High] Buffer overflow and Fortify Source panic when reading `calib_sensitivities` for devices with more than 8 channels.
--

commit 837f207153012dfc0860b6d46ad5d3ae4f7b3004
Author: Jun Yan <jerrysteve1101@gmail.com>

Extend cap11xx_hw_model structure to support CAP1114

This patch extends the cap11xx_hw_model structure to support CAP1114
with different register offsets and hardware characteristics.

> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 1db4a9090705..0f19ee036e78 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c

[ ... ]

> @@ -211,8 +215,8 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
>  	}
>  
>  	if (!of_property_read_u32_array(node, "microchip,input-threshold",
> -					priv->thresholds, priv->model->num_channels)) {
> +					priv->thresholds, priv->model->num_sensor_thresholds)) {
> -		for (i = 0; i < priv->model->num_channels; i++) {
> +		for (i = 0; i < priv->model->num_sensor_thresholds; i++) {
>  			if (priv->thresholds[i] > 127) {
>  				dev_err(dev, "Invalid input-threshold value %u\n",
>  					priv->thresholds[i]);

[Severity: High]
This is a pre-existing issue, but looking at cap11xx_init_keys(), could
reading "microchip,calib-sensitivity" overflow the calib_sensitivities array
for devices with more than 8 channels?

In cap11xx_init_keys():

    } else if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
                                           priv->calib_sensitivities,
                                           priv->model->num_channels)) {

The calib_sensitivities array is statically allocated to 8 elements in
struct cap11xx_priv. For models with more than 8 channels (like the CAP1114
which has 14), could this cause a buffer overflow into the keycodes array
and trigger a kernel panic if FORTIFY_SOURCE is enabled?

[ ... ]

> @@ -308,7 +314,7 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
>  	if (ret < 0)
>  		goto out;
>  
> -	ret = regmap_read(priv->regmap, CAP11XX_REG_SENSOR_INPUT, &status);
> +	ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base, &status);
>  	if (ret < 0)
>  		goto out;

[Severity: High]
Will this result in reading stale cached values instead of the actual
hardware state?

The interrupt handler now dynamically reads from sensor_input_reg_base.
However, cap11xx_volatile_reg() still hardcodes CAP11XX_REG_SENSOR_INPUT as
a volatile register.

Since the new dynamic base register (and the subsequent register for devices
with more than 8 channels) is no longer marked volatile, will the regmap
cache return stale data and break input events for these models?

[ ... ]

> @@ -362,7 +368,7 @@ static int cap11xx_led_set(struct led_classdev *cdev,
>  	 * 0 (OFF) and 1 (ON).
>  	 */
>  	return regmap_update_bits(priv->regmap,
> -				  CAP11XX_REG_LED_OUTPUT_CONTROL,
> +				  priv->model->led_output_control_reg_base,
>  				  BIT(led->reg),
>  				  value ? BIT(led->reg) : 0);
>  }

[Severity: High]
Will this bitmask be truncated for LEDs with an index of 8 or higher?

The bitmask passed to regmap_update_bits() is BIT(led->reg). Since models
like CAP1114 support more than 8 LEDs, and the register operations are 8-bit,
BIT(led->reg) will exceed 8 bits for higher-index LEDs.

The initialization code in cap11xx_init_leds() caps the LED clearance mask to
8 bits:

    GENMASK(min(num_leds, 8) - 1, 0)

Does cap11xx_led_set() also need an update to handle devices with more than
8 LEDs to avoid silently failing?

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