[PATCH] Input: stmpe-keypad - disable keypad on probe failure

[PATCH] Input: stmpe-keypad - disable keypad on probe failure

[Myeonghun Pak]

stmpe_keypad_chip_init() enables the keypad block before programming
its registers. If any subsequent register access fails, the function
returns without disabling the block. The later IRQ request and input
device registration failures have the same problem.

Route every failure after a successful stmpe_enable() through a cleanup
path so a failed probe does not leave the keypad block enabled.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: 76f10845318b ("input: Add STMPE keypad driver")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/input/keyboard/stmpe-keypad.c | 38 ++++++++++++++++++---------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/stmpe-keypad.c b/drivers/input/keyboard/stmpe-keypad.c
index 0acded4fb9c9c..58a5ce4eaa80b 100644
--- a/drivers/input/keyboard/stmpe-keypad.c
+++ b/drivers/input/keyboard/stmpe-keypad.c
@@ -280,35 +280,44 @@ static int stmpe_keypad_chip_init(struct stmpe_keypad *keypad)
 
 	ret = stmpe_keypad_altfunc_init(keypad);
 	if (ret < 0)
-		return ret;
+		goto disable_keypad;
 
 	ret = stmpe_reg_write(stmpe, STMPE_KPC_COL, keypad->cols);
 	if (ret < 0)
-		return ret;
+		goto disable_keypad;
 
 	ret = stmpe_reg_write(stmpe, STMPE_KPC_ROW_LSB, keypad->rows);
 	if (ret < 0)
-		return ret;
+		goto disable_keypad;
 
 	if (variant->max_rows > 8) {
 		ret = stmpe_set_bits(stmpe, STMPE_KPC_ROW_MSB,
 				     STMPE_KPC_ROW_MSB_ROWS,
 				     keypad->rows >> 8);
 		if (ret < 0)
-			return ret;
+			goto disable_keypad;
 	}
 
 	ret = stmpe_set_bits(stmpe, STMPE_KPC_CTRL_MSB,
 			     STMPE_KPC_CTRL_MSB_SCAN_COUNT,
 			     keypad->scan_count << 4);
 	if (ret < 0)
-		return ret;
+		goto disable_keypad;
+
+	ret = stmpe_set_bits(stmpe, STMPE_KPC_CTRL_LSB,
+			     STMPE_KPC_CTRL_LSB_SCAN |
+			     STMPE_KPC_CTRL_LSB_DEBOUNCE,
+			     STMPE_KPC_CTRL_LSB_SCAN |
+			     (keypad->debounce_ms << 1));
+	if (ret < 0)
+		goto disable_keypad;
 
-	return stmpe_set_bits(stmpe, STMPE_KPC_CTRL_LSB,
-			      STMPE_KPC_CTRL_LSB_SCAN |
-			      STMPE_KPC_CTRL_LSB_DEBOUNCE,
-			      STMPE_KPC_CTRL_LSB_SCAN |
-			      (keypad->debounce_ms << 1));
+	return 0;
+
+disable_keypad:
+	stmpe_disable(stmpe, STMPE_BLOCK_KEYPAD);
+
+	return ret;
 }
 
 static void stmpe_keypad_fill_used_pins(struct stmpe_keypad *keypad,
@@ -389,19 +398,24 @@ static int stmpe_keypad_probe(struct platform_device *pdev)
 					  IRQF_ONESHOT, "stmpe-keypad", keypad);
 	if (error) {
 		dev_err(&pdev->dev, "unable to get irq: %d\n", error);
-		return error;
+		goto disable_keypad;
 	}
 
 	error = input_register_device(input);
 	if (error) {
 		dev_err(&pdev->dev,
 			"unable to register input device: %d\n", error);
-		return error;
+		goto disable_keypad;
 	}
 
 	platform_set_drvdata(pdev, keypad);
 
 	return 0;
+
+disable_keypad:
+	stmpe_disable(stmpe, STMPE_BLOCK_KEYPAD);
+
+	return error;
 }
 
 static void stmpe_keypad_remove(struct platform_device *pdev)
-- 
2.47.1

Re: [PATCH] Input: stmpe-keypad - disable keypad on probe failure

[sashiko-bot]

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

New issues:
- [Medium] Disabling hardware before freeing managed IRQ creates a race condition on probe failure.

Pre-existing issues:
- [High] Logic bug in `stmpe_keypad_altfunc_init()` aborts pull-up configuration on non-zero register values and ignores errors.
- [High] Heap buffer overflow in `keymap` parsing when device tree specifies dimensions exceeding 8x8.
--

--- Patch [1]: [PATCH] Input: stmpe-keypad - disable keypad on probe failure ---
commit 849a668c20609a76cff8c0630950b6029798e3f9
Author: Myeonghun Pak <mhun512@gmail.com>
Subject: Input: stmpe-keypad - disable keypad on probe failure

stmpe_keypad_chip_init() enables the keypad block before programming
its registers, but does not disable it on failure. This patch routes
initialization and probe failures through a cleanup path so a failed
probe does not leave the keypad block enabled.

Fixes: 76f10845318b ("input: Add STMPE keypad driver")
Cc: stable@vger.kernel.org

> diff --git a/drivers/input/keyboard/stmpe-keypad.c b/drivers/input/keyboard/stmpe-keypad.c
> --- a/drivers/input/keyboard/stmpe-keypad.c
> +++ b/drivers/input/keyboard/stmpe-keypad.c
[ ... ]
> @@ -280,35 +280,44 @@ static int stmpe_keypad_chip_init(struct stmpe_keypad *keypad)
>  
>  	ret = stmpe_keypad_altfunc_init(keypad);
>  	if (ret < 0)
> -		return ret;
> +		goto disable_keypad;
>  

[Severity: High]
This is a pre-existing issue, but does this logic correctly handle the return
value of the configuration function?

Looking at stmpe_keypad_altfunc_init(), it reads a register which returns a
positive value on success, but it treats any non-zero value as an error:

drivers/input/keyboard/stmpe-keypad.c:stmpe_keypad_altfunc_init() {
    ...
    ret = stmpe_reg_read(stmpe, pureg);
    if (ret)
        return ret;
    ...
}

This causes stmpe_keypad_altfunc_init() to return a positive value, skipping
the rest of the pull-up configuration. Because the check here in
stmpe_keypad_chip_init() only looks for (ret < 0), the positive error code is
silently ignored, leaving the matrix columns floating.

[ ... ]
> @@ -389,19 +398,24 @@ static int stmpe_keypad_probe(struct platform_device *pdev)
>  					  IRQF_ONESHOT, "stmpe-keypad", keypad);
>  	if (error) {
>  		dev_err(&pdev->dev, "unable to get irq: %d\n", error);
> -		return error;
> +		goto disable_keypad;
>  	}
>  
>  	error = input_register_device(input);
>  	if (error) {
>  		dev_err(&pdev->dev,
>  			"unable to register input device: %d\n", error);
> -		return error;
> +		goto disable_keypad;
>  	}

[Severity: High]
This is a pre-existing issue, but could the matrix_keypad_build_keymap()
call earlier in this probe function cause a heap buffer overflow?

The keypad->keymap array is statically sized to 64 elements
(STMPE_KEYPAD_MAX_ROWS * STMPE_KEYPAD_MAX_COLS). However, the device tree
parsing does not validate that the parsed rows and cols fit within these 8x8
bounds.

If a variant like STMPE24xx supports and specifies 12 rows in the device
tree, won't matrix_keypad_build_keymap() write up to 96 elements into the
64-element keypad->keymap array?

>  
>  	platform_set_drvdata(pdev, keypad);
>  
>  	return 0;
> +
> +disable_keypad:
> +	stmpe_disable(stmpe, STMPE_BLOCK_KEYPAD);
> +
> +	return error;
>  }

[Severity: Medium]
Because the IRQ is managed by devres via devm_request_threaded_irq(), it
won't be freed until after the probe function returns. By manually disabling
the keypad block here, does this introduce a race condition?

If an interrupt fires before devres cleans up the IRQ, won't the interrupt
handler (stmpe_keypad_irq) execute and attempt to read from the disabled
hardware registers?

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

Re: [PATCH] Input: stmpe-keypad - disable keypad on probe failure

[Dmitry Torokhov]

Hi Myeonghun,

On Tue, Aug 04, 2026 at 11:15:25PM +0900, Myeonghun Pak wrote:
> stmpe_keypad_chip_init() enables the keypad block before programming
> its registers. If any subsequent register access fails, the function
> returns without disabling the block. The later IRQ request and input
> device registration failures have the same problem.

What exactly is the problem with leaving keyboard block enabled? 

Thanks.

-- 
Dmitry