Re: [PATCH] Input: libps2 - annotate lockless command wait conditions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] Input: libps2 - unannotated lockless wait condition in
  ps2_drain()
--

commit 9801ff26658df02a74329e6bb2b403d12fd15880
Author: Zhu Jun <zhujun2@cmss.chinamobile.com>

Input: libps2 - annotate lockless command wait conditions

This commit introduces ps2_is_waiting() to annotate lockless wait conditions
on ps2dev->flags and ps2dev->cmdcnt with data_race(READ_ONCE(...)).

> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index 05b64277aecd9..a60912224bfac 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -41,6 +41,12 @@
>  #define PS2_FLAG_NAK		BIT(4)	/* Last transmission was NAKed */
>  #define PS2_FLAG_PASS_NOACK	BIT(5)	/* Pass non-ACK byte to receive handler */
>  
> +static bool ps2_is_waiting(struct ps2dev *ps2dev, unsigned long flag)
> +{
> +	/* Waiters sample flags while the receive interrupt updates them. */
> +	return data_race(READ_ONCE(ps2dev->flags)) & flag;
> +}
> +
>  static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
>  			   unsigned int timeout, unsigned int max_attempts)
>  	__releases(&ps2dev->serio->lock) __acquires(&ps2dev->serio->lock)
> @@ -62,7 +68,7 @@ static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
>  				"failed to write %#02x: %d\n", byte, error);
>  		else
>  			wait_event_timeout(ps2dev->wait,
> -					   !(ps2dev->flags & PS2_FLAG_ACK),
> +					   !ps2_is_waiting(ps2dev, PS2_FLAG_ACK),
>  					   msecs_to_jiffies(timeout));
>  
>  		serio_pause_rx(ps2dev->serio);

[Severity: Low]
Should ps2_drain() also use !ps2_is_waiting(ps2dev, PS2_FLAG_CMD) for its
wait_event_timeout() condition?

drivers/input/serio/libps2.c:ps2_drain() {
	...
	scoped_guard(serio_pause_rx, ps2dev->serio) {
		ps2dev->flags = PS2_FLAG_CMD;
		ps2dev->cmdcnt = maxbytes;
	}

	wait_event_timeout(ps2dev->wait,
			   !(ps2dev->flags & PS2_FLAG_CMD),
			   msecs_to_jiffies(timeout));
}

Like ps2_do_sendbyte() and __ps2_command(), ps2_drain() drops
ps2dev->serio->lock before waiting for the receive interrupt handler to
clear PS2_FLAG_CMD, leaving an unannotated data race on ps2dev->flags.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111084142.80954-1-zhujun2@cmss.chinamobile.com?part=1