Re: [PATCHv2] input: ipaq-micro-keys: simplify allocation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Potential deadlock in `micro_key_start()` and `micro_key_stop()` due to taking a spinlock without disabling interrupts, which is also taken in a hardirq handler.
--

commit 59a420f5a26f394b8f84d4e056f388e517d6f808
Author: Rosen Penev <rosenp@gmail.com>

input: ipaq-micro-keys: simplify allocation

This commit simplifies the keycode array allocation in the ipaq-micro-keys
driver. By embedding the array directly into the device structure, it removes
the need for a separate devm_kmemdup_array() call during probe.

> diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
> index ca7ec054b1cef..695ef3c2081a4 100644
> --- a/drivers/input/keyboard/ipaq-micro-keys.c
> +++ b/drivers/input/keyboard/ipaq-micro-keys.c

[ ... ]

> @@ -102,10 +102,7 @@ static int micro_key_probe(struct platform_device *pdev)
>  
>  	keys->input->keycodesize = sizeof(micro_keycodes[0]);
>  	keys->input->keycodemax = ARRAY_SIZE(micro_keycodes);
> -	keys->codes = devm_kmemdup_array(&pdev->dev, micro_keycodes, keys->input->keycodemax,
> -					 keys->input->keycodesize, GFP_KERNEL);
> -	if (!keys->codes)
> -		return -ENOMEM;
> +	memcpy(keys->codes, micro_keycodes, sizeof(keys->codes));
>  
>  	keys->input->keycode = keys->codes;

[Severity: High]
This isn't a bug introduced by this patch, but could the lock usage in
micro_key_start() and micro_key_stop() cause a deadlock if an interrupt
fires?

Looking at drivers/input/keyboard/ipaq-micro-keys.c, the micro->lock is
acquired in process context without disabling interrupts:

micro_key_start() {
    guard(spinlock)(&keys->micro->lock);
    ...
}

If a serial interrupt fires on the same CPU while the process context holds
the lock, the interrupt handler micro_serial_isr() runs, processes the
character, and calls micro_rx_msg().

micro_rx_msg() in drivers/mfd/ipaq-micro.c then attempts to acquire the same
lock in hardirq context:

    spin_lock(&micro->lock);

Does this sequence lead to a lock inversion deadlock? Should the process
context functions use guard(spinlock_irqsave) or guard(spinlock_irq)
instead?

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