Thread (2 messages) flat view 2 messages, 2 authors, 1d ago

Re: [PATCH 2/2] input: fix repeated words in comments

From: sashiko-bot@kernel.org
Date: 2026-09-04 11:32:36

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

New issues:
- [Low] The patch incorrectly fixes a typo, resulting in a grammatically incomplete sentence.

Pre-existing issues:
- [High] Repeated firmware updates via sysfs leak `input_dev` structures and devres nodes due to manual unregistration of a devm-managed device without freeing it.
- [High] The `serio->lock` spinlock is accessed uninitialized if an interrupt fires before `serio_register_port()` completes.
- [High] A Use-After-Free can occur on `q40kbd->port` during device removal if a pending interrupt fires before `free_irq()`.
--

commit 45b867db64f6ee078771ef7b8cf723b44cd0fd23
Author: Hemanth Selam [off-list ref]

    input: fix repeated words in comments
    
    Drop words accidentally written twice, reported by checkpatch.pl as a
    possible repeated word.  Only touches comments, no code changes.
    
    Assisted-by: Cursor:claude-opus-5
    Signed-off-by: Hemanth Selam [off-list ref]

[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/serio/q40kbd.c b/drivers/input/serio/q40kbd.c
index 2f553efbe649d..7ab4db61e46fe 100644
--- a/drivers/input/serio/q40kbd.c
+++ b/drivers/input/serio/q40kbd.c
@@ -72,7 +72,7 @@ static void q40kbd_stop(void)
 
 /*
  * q40kbd_open() is called when a port is open by the higher layer.
- * It allocates the interrupt and enables in in the chip.
+ * It allocates the interrupt and enables in the chip.
[Severity: Low]
Does this typo fix leave the sentence grammatically incomplete? The original
text "enables in in the chip" appears to be a typo for "enables it in the
chip".
  */
 
 static int q40kbd_open(struct serio *port)
[Severity: High]
This is a pre-existing issue, but does cyapa_update_fw_store() leak memory and
devres tracking nodes on every firmware update?

drivers/input/mouse/cyapa.c:cyapa_update_fw_store() {
    ...
    input_unregister_device(cyapa->input);
    cyapa->input = NULL;
    ...
}

Manual unregistration of a devm-allocated device without freeing it leaves the
devres tracking node and memory orphaned. When cyapa_reinitialize() is called,
it allocates a new input device, creating a new devres node.


[Severity: High]
This is a pre-existing issue, but can the serio port spinlock be accessed
uninitialized if an interrupt fires before q40kbd_probe() completes?

drivers/input/serio/q40kbd.c:q40kbd_probe() {
    ...
    error = request_irq(Q40_IRQ_KEYBOARD, q40kbd_interrupt, 0,
                        DRV_NAME, q40kbd);
    ...
    serio_register_port(q40kbd->port);
    ...
}

If an interrupt fires when request_irq() is called, q40kbd_interrupt() will
call serio_interrupt(), which acquires serio->lock. However, the spinlock
isn't initialized until serio_register_port() is called.


[Severity: High]
This is a pre-existing issue, but is there a use-after-free race during
device removal if a pending interrupt fires before the IRQ is freed?

drivers/input/serio/q40kbd.c:q40kbd_remove() {
    ...
    serio_unregister_port(q40kbd->port);
    free_irq(Q40_IRQ_KEYBOARD, q40kbd);
    ...
}

While serio_unregister_port() calls the close method to mask the hardware
interrupt, it does not wait for already-running handlers. If
q40kbd_interrupt() is executing on another CPU, it can dereference
q40kbd->port after it has been freed by serio_unregister_port().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904111911.32224-1-hemanth.selam@gmail.com?part=2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help