[PATCH v2] Input: soc_button_array - fix MS Surface Pro 11 probe failure

[PATCH v2] Input: soc_button_array - fix MS Surface Pro 11 probe failure

[Hans de Goede]

On the MS Surface Pro 11 soc_button_array probing races with the GPIO
driver probing. If soc_button_array wins the race then gpiod_get() returns
EPROBE_DEFER, which should normally take care of retrying later, but
the soc_button_array code deliberately ignores EPROBE_DEFER causing it
to fail its probe() which causes the volume and power buttons to now work.

The ignoring of EPROBE_DEFER is there to deal with a problem specific to
older Bay Trail (BYT) and Cherry Trail (CHT) tablets which often use this
driver. Modify the error handling to only ignore EPROBE_DEFER on BYT and
CHT platforms and propagate EPROBE_DEFER normally on other platforms.

Reported-by: Sergey Lebedev <lsa.uz@pm.me>
Closes: https://lore.kernel.org/lkml/20260830141355.55898-1-lsa.uz@pm.me/
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
Changes in v2:
- Also check for irq == -EPROBE_DEFER (Shashiko)
- Drop #ifdef X86-ified soc_intel_is_byt_or_cht() helper,
  linux/platform_data/x86/soc.h already contains non x86 stubs (Shashiko)
---
 drivers/input/misc/soc_button_array.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index b8cad415c62c..f838b7d72e49 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -16,6 +16,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/gpio_keys.h>
 #include <linux/gpio.h>
+#include <linux/platform_data/x86/soc.h>
 #include <linux/platform_device.h>
 
 static bool use_low_level_irq;
@@ -191,8 +192,9 @@ soc_button_device_create(struct platform_device *pdev,
 		error = soc_button_lookup_gpio(&pdev->dev, info->acpi_index, &gpio, &irq);
 		if (error || irq < 0) {
 			/*
-			 * Skip GPIO if not present. Note we deliberately
-			 * ignore -EPROBE_DEFER errors here. On some devices
+			 * Propagate -EPROBE_DEFER, skip button on other errors.
+			 *
+			 * -EPROBE_DEFER is ignored on Bay & Cherry Trail. Here
 			 * Intel is using so called virtual GPIOs which are not
 			 * GPIOs at all but some way for AML code to check some
 			 * random status bits without need a custom opregion.
@@ -201,6 +203,12 @@ soc_button_device_create(struct platform_device *pdev,
 			 * we do not have a driver for these so they will never
 			 * show up, therefore we ignore -EPROBE_DEFER.
 			 */
+			if ((error == -EPROBE_DEFER || irq == -EPROBE_DEFER) &&
+			    !(soc_intel_is_byt() || soc_intel_is_cht())) {
+				error = -EPROBE_DEFER;
+				goto err_free_mem;
+			}
+
 			continue;
 		}
 
-- 
2.55.0

Re: [PATCH v2] Input: soc_button_array - fix MS Surface Pro 11 probe failure

[sashiko-bot]

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

New issues:
- [High] The local variable `irq` is evaluated without being initialized when `soc_button_lookup_gpio()` returns an error other than `-EPROBE_DEFER` (e.g., `-ENOENT`).

Pre-existing issues:
- [High] Missing bounds check on `btns_desc->package.count` before accessing the first element of the ACPI package.
--

commit 71e6e56f2553b9410815c77e8b20ed9b2b9a2d37
Author: Hans de Goede <johannes.goede@oss.qualcomm.com>

Input: soc_button_array - fix MS Surface Pro 11 probe failure

This commit alters the error handling in soc_button_device_create() to
propagate EPROBE_DEFER on platforms other than older Bay Trail and Cherry
Trail tablets. This allows the MS Surface Pro 11 to correctly retry probing
when racing with the GPIO driver.

Link: https://lore.kernel.org/lkml/20260830141355.55898-1-lsa.uz@pm.me/

> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index b8cad415c62ca..f838b7d72e49f 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
[ ... ]
> @@ -191,8 +192,9 @@ soc_button_device_create(struct platform_device *pdev,
>  		error = soc_button_lookup_gpio(&pdev->dev, info->acpi_index, &gpio, &irq);
>  		if (error || irq < 0) {
>  			/*
> -			 * Skip GPIO if not present. Note we deliberately
> -			 * ignore -EPROBE_DEFER errors here. On some devices
> +			 * Propagate -EPROBE_DEFER, skip button on other errors.
> +			 *
> +			 * -EPROBE_DEFER is ignored on Bay & Cherry Trail. Here
>  			 * Intel is using so called virtual GPIOs which are not
>  			 * GPIOs at all but some way for AML code to check some
>  			 * random status bits without need a custom opregion.
> @@ -201,6 +203,12 @@ soc_button_device_create(struct platform_device *pdev,
>  			 * we do not have a driver for these so they will never
>  			 * show up, therefore we ignore -EPROBE_DEFER.
>  			 */
> +			if ((error == -EPROBE_DEFER || irq == -EPROBE_DEFER) &&

[Severity: High]
Can this evaluate the uninitialized local variable irq?

When soc_button_lookup_gpio() encounters an error other than -EPROBE_DEFER,
it returns early without setting irq_ret:

soc_button_lookup_gpio()
    desc = gpiod_get_index(dev, NULL, acpi_index, GPIOD_ASIS);
    if (IS_ERR(desc))
        return PTR_ERR(desc);

If error is -ENOENT, the short-circuiting of the || operator means the
condition evaluates irq == -EPROBE_DEFER while irq is uninitialized stack
memory.

This will trigger KMSAN warnings, and if the uninitialized memory happens to
contain -517 (-EPROBE_DEFER), it would falsely abort the entire probe instead
of just skipping the missing button.

> +			    !(soc_intel_is_byt() || soc_intel_is_cht())) {
> +				error = -EPROBE_DEFER;
> +				goto err_free_mem;
> +			}
> +
>  			continue;
>  		}

This is a pre-existing issue, but does soc_button_get_button_info() lack a
bounds check on the ACPI package element count?

soc_button_get_button_info()
    /* The first package describes the collection */
    el0 = &btns_desc->package.elements[0];
    if (el0->type == ACPI_TYPE_PACKAGE &&

If the system provides a malformed ACPI table with an empty package where
btns_desc->package.count is 0, this will access out-of-bounds memory or
dereference a NULL pointer, potentially causing a panic during boot.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909090721.20869-1-johannes.goede@oss.qualcomm.com?part=1