Re: [PATCH v2] tty: n_tty: annotate lockless read of ldata->icanon in input_available_p()

[Greg Kroah-Hartman]

On Tue, Mar 17, 2026 at 04:19:11PM +0800, Ziyu Zhang wrote:
> n_tty_poll() calls input_available_p() without holding termios_rwsem to
> check input readiness for select()/poll(). input_available_p() reads
> ldata->icanon, which can be concurrently written by n_tty_set_termios()
> under down_write(termios_rwsem).
> 
> This is a benign race: poll/select readiness is best-effort, and the
> actual n_tty_read() path re-checks icanon under down_read(termios_rwsem).
> A stale icanon value in poll only causes a transiently incorrect
> readiness result, which is permitted by POSIX poll/select semantics.
> 
> Since icanon is a bitfield, READ_ONCE()/WRITE_ONCE() cannot be used.
> Annotate the read with data_race() and evaluate it as a boolean to document
> the intentional lockless access, bypass __auto_type compiler errors, and
> suppress data race detector warnings.
> 
> Reported-by: kernel test robot <lkp@xxxxxxxxx>

No this robot did not report this issue :(

> Closes: https://lore.kernel.org/oe-kbuild-all/202603162328.vY9JOJWL-lkp@xxxxxxxxx/
> Signed-off-by: Ziyu Zhang <ziyuzhang201@xxxxxxxxx>
> ---
>  drivers/tty/n_tty.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index e6a0f5b40..0a0d8d70c 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1909,7 +1909,8 @@ static inline int input_available_p(const struct tty_struct *tty, int poll)
>  	const struct n_tty_data *ldata = tty->disc_data;
>  	int amt = poll && !TIME_CHAR(tty) && MIN_CHAR(tty) ? MIN_CHAR(tty) : 1;
>  
> -	if (ldata->icanon && !L_EXTPROC(tty))
> +	/* data_race: benign race, poll readiness is best-effort */
> +	if (data_race(!!ldata->icanon) && !L_EXTPROC(tty))
>  		return ldata->canon_head != ldata->read_tail;

Given that the previous patch was not even tested, how was this found
and most importantly, tested?  What error was reported that this now
fixes?

What changed to cause this to now be needed?

thanks,

greg k-h