Re: [PATCH] Input: ipaq-micro-keys - add length check in micro_key_receive

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] `micro_key_start()` and `micro_key_stop()` acquire `keys->micro->lock` without disabling interrupts, leading to a potential deadlock with the MFD serial interrupt handler.
--

commit 33b9102060e60926d04c255f54b3c13bbafe5ca3
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

    Input: ipaq-micro-keys - add length check in micro_key_receive
    
    The driver accesses the message payload (msg[0]) without checking if
    the length is greater than zero. The parent MFD driver can produce a
    payload with a length of 0, in which case msg[0] would be uninitialized
    or stale.
    
    Add a check to return early if len is less than 1.
    
    Reported-by: sashiko-bot@kernel.org
    Assisted-by: Antigravity:gemini-3.5-flash
    Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
> index 695ef3c2081a4..b2068fbc82651 100644
> --- a/drivers/input/keyboard/ipaq-micro-keys.c
> +++ b/drivers/input/keyboard/ipaq-micro-keys.c
> @@ -43,6 +43,9 @@ static void micro_key_receive(void *data, int len, unsigned char *msg)
>  	struct ipaq_micro_keys *keys = data;
>  	int key, down;
>  
> +	if (len < 1)
> +		return;
> +
>  	down = 0x80 & msg[0];
>  	key  = 0x7f & msg[0];
>  

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential deadlock
involving keys->micro->lock when looking at the surrounding code.

Does the use of guard(spinlock) in micro_key_start() and micro_key_stop()
risk a deadlock with the MFD serial interrupt handler?

drivers/input/keyboard/ipaq-micro-keys.c:micro_key_start() {
	guard(spinlock)(&keys->micro->lock);
	...
}

drivers/input/keyboard/ipaq-micro-keys.c:micro_key_stop() {
	guard(spinlock)(&keys->micro->lock);
	...
}

The keys->micro->lock spinlock is also acquired within the MFD driver's hard
IRQ handler in micro_serial_isr(). Since micro_key_start() and
micro_key_stop() are called in process context without disabling local
interrupts, could an interrupt firing on the same CPU while the lock is held
cause the ISR to try acquiring the lock again and hang?

Should these functions be using guard(spinlock_irqsave) or equivalent to
prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/aintAvTyw4CVb5hG@google.com?part=1