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

[sashiko-bot@kernel.org]

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 <hemanth.selam@gmail.com>

    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 <hemanth.selam@gmail.com>

[ ... ]

> 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