Skip to contentSkip to conversation

Input: ipaq-micro-keys - add length check in micro_key_receive

PATCH New Dmitry Torokhov 2 participants 2 messages Jun 10

Subsystems (2)
HID Input drivers
lore
Share & tools
lore

Dmitry Torokhov

author maintainer
#0 lore raw

PATCH 1/1 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 [email protected]
Assisted-by Antigravity:gemini-3.5-flash
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/input/keyboard/ipaq-micro-keys.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/input/keyboard/ipaq-micro-keys.c +3 -0
index 3c7d6aa0fe29..ebd991de70f8 100644
@@ -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];
Signature · 5 lines
2.54.0.1099.g489fc7bff1-goog


-- 
Dmitry
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

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 <[email protected]>

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 [email protected]
Assisted-by Antigravity:gemini-3.5-flash
Signed-off-by Dmitry Torokhov <[email protected]>
Quoted 5 lines
drivers/input/keyboard/ipaq-micro-keys.c +0 -0
index 695ef3c2081a4..b2068fbc82651 100644

drivers/input/keyboard/ipaq-micro-keys.c static void micro_key_receive(void *data, int len, unsigned char *msg) · line 51

Quoted 9 lines
drivers/input/keyboard/ipaq-micro-keys.c +3 -0
@@ -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?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1