Re: [PATCH 4/7] Input: gscps2 - serialize hardware and buffer access in gscps2_flush()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] gscps2_writeb_output spins infinitely if the hardware receive buffer becomes full while executing in a context where IRQs are disabled.
--

commit 98b3bfa0449cf3b48806ab726d6170adf6e0246e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

    Input: gscps2 - serialize hardware and buffer access in gscps2_flush()
    
    gscps2_flush() reads from hardware registers and resets the ring buffer
    indices ps2port->act and ps2port->append. In gscps2_enable(), the
    trailing gscps2_flush() was called without holding ps2port->lock, racing
    with concurrent hardware interrupts and buffer access.
    
    Assert that ps2port->lock is held in gscps2_flush() with
    lockdep_assert_held(), and ensure all callers acquire ps2port->lock so
    that multi-step hardware sequences remain fully serialized without
    unprotected windows.

> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index fef6fffb6f865..36c25db9ff7f6 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c
[ ... ]
> @@ -364,7 +366,8 @@ static void gscps2_enable(struct gscps2port *ps2port, bool enable)

[Severity: High]
This is a pre-existing issue, but can the loop in the preceding function
gscps2_writeb_output() spin infinitely and cause a hard lockup?

Looking at gscps2_writeb_output():

	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
		cpu_relax();

This lockless loop waits for a status flag to clear without a timeout or
fallback wake mechanism. If a byte arrives from the PS/2 port (setting
GSC_STAT_RBNE) while gscps2_writeb_output() is executing with local
interrupts disabled, the gscps2_interrupt handler cannot run on the same CPU
to drain the buffer.

Could this spin forever if triggered by normal hardware interaction, such as
typing on the keyboard while the system sends a command?

>  	}
>  
>  	wait_TBE(ps2port->addr);
> -	gscps2_flush(ps2port);
> +	scoped_guard(spinlock_irqsave, &ps2port->lock)
> +		gscps2_flush(ps2port);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-gscps2-v1-0-c733d4cae7f9@gmail.com?part=4