[PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl

Osama Abdelkader posted 1 patch 1 month, 2 weeks ago
drivers/tty/n_tty.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl
Posted by Osama Abdelkader 1 month, 2 weeks ago
n_tty_lookahead_flow_ctrl() accesses ldata->lookahead_count without
holding termios_rwsem, while reset_buffer_flags() in n_tty_flush_buffer()
resets it with exclusive termios_rwsem held. This causes a data race
reported by KCSAN when a PTY is closed while flush_to_ldisc is still
processing lookahead data.

Fix by taking termios_rwsem (read) in n_tty_lookahead_flow_ctrl(),
consistent with __receive_buf() which also modifies lookahead_count
under the read lock.

Reported-by: syzbot+80806cf7508e92c7cc86@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=80806cf7508e92c7cc86
Fixes: 6bb6fa6908eb ("tty: Implement lookahead to process XON/XOFF timely")
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/tty/n_tty.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e6a0f5b40d0a..725d6ed2b863 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1480,6 +1480,8 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const u8 *cp,
 	struct n_tty_data *ldata = tty->disc_data;
 	u8 flag = TTY_NORMAL;
 
+	guard(rwsem_read)(&tty->termios_rwsem);
+
 	ldata->lookahead_count += count;
 
 	if (!I_IXON(tty))
-- 
2.43.0
Re: [PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl
Posted by Greg Kroah-Hartman 3 weeks ago
On Wed, Feb 11, 2026 at 10:08:38PM +0100, Osama Abdelkader wrote:
> n_tty_lookahead_flow_ctrl() accesses ldata->lookahead_count without
> holding termios_rwsem, while reset_buffer_flags() in n_tty_flush_buffer()
> resets it with exclusive termios_rwsem held. This causes a data race
> reported by KCSAN when a PTY is closed while flush_to_ldisc is still
> processing lookahead data.

A data race of what exactly?  lookahead_count?

> Fix by taking termios_rwsem (read) in n_tty_lookahead_flow_ctrl(),
> consistent with __receive_buf() which also modifies lookahead_count
> under the read lock.

This feels wrong.  I would like to see a LOT of testing and validation
that this is correct before being able to take this patch.  How was that
done?

thanks,

greg k-h
Re: [PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl
Posted by Osama Abdelkader 2 weeks, 5 days ago
On Thu, Mar 12, 2026 at 03:21:03PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Feb 11, 2026 at 10:08:38PM +0100, Osama Abdelkader wrote:
> > n_tty_lookahead_flow_ctrl() accesses ldata->lookahead_count without
> > holding termios_rwsem, while reset_buffer_flags() in n_tty_flush_buffer()
> > resets it with exclusive termios_rwsem held. This causes a data race
> > reported by KCSAN when a PTY is closed while flush_to_ldisc is still
> > processing lookahead data.
> 
> A data race of what exactly?  lookahead_count?

yes, ldata->lookahead_count.

> 
> > Fix by taking termios_rwsem (read) in n_tty_lookahead_flow_ctrl(),
> > consistent with __receive_buf() which also modifies lookahead_count
> > under the read lock.
> 
> This feels wrong.  I would like to see a LOT of testing and validation
> that this is correct before being able to take this patch.  How was that
> done?
>

To clarify the reported race:

The race is on `ldata->lookahead_count` between:

1. `n_tty_lookahead_flow_ctrl()`, which does `lookahead_count += count`
   from the `flush_to_ldisc()` workqueue path:

   `flush_to_ldisc() -> lookahead_bufs() -> tty_port_default_lookahead_buf() -> n_tty_lookahead_flow_ctrl()`

2. `reset_buffer_flags()`, which does `lookahead_count = 0`
   from `n_tty_flush_buffer()` under write `termios_rwsem`
   (for example during PTY close / hangup).

`__receive_buf()` already accesses the same field under read `termios_rwsem`,
so this lookahead path was the remaining unlocked access to that state.

Because `n_tty_lookahead_flow_ctrl()` performs a read-modify-write (`+= count`),
`READ_ONCE()/WRITE_ONCE()` would still allow lost updates, so taking read
`termios_rwsem` there matches the existing writer-side protection.

I also ran the change with lockdep enabled and did not observe lockdep warnings
on PTY open/close stress.
 
> thanks,
> 
> greg k-h

Thanks,
Osama
Re: [PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl
Posted by Osama Abdelkader 3 weeks, 6 days ago
On Wed, Feb 11, 2026 at 10:08:38PM +0100, Osama Abdelkader wrote:
> n_tty_lookahead_flow_ctrl() accesses ldata->lookahead_count without
> holding termios_rwsem, while reset_buffer_flags() in n_tty_flush_buffer()
> resets it with exclusive termios_rwsem held. This causes a data race
> reported by KCSAN when a PTY is closed while flush_to_ldisc is still
> processing lookahead data.
> 
> Fix by taking termios_rwsem (read) in n_tty_lookahead_flow_ctrl(),
> consistent with __receive_buf() which also modifies lookahead_count
> under the read lock.
> 
> Reported-by: syzbot+80806cf7508e92c7cc86@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=80806cf7508e92c7cc86
> Fixes: 6bb6fa6908eb ("tty: Implement lookahead to process XON/XOFF timely")
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
>  drivers/tty/n_tty.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index e6a0f5b40d0a..725d6ed2b863 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1480,6 +1480,8 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const u8 *cp,
>  	struct n_tty_data *ldata = tty->disc_data;
>  	u8 flag = TTY_NORMAL;
>  
> +	guard(rwsem_read)(&tty->termios_rwsem);
> +
>  	ldata->lookahead_count += count;
>  
>  	if (!I_IXON(tty))
> -- 
> 2.43.0
> 

Hi All,

Can you please review?
Thanks for your time in advance.

BR,
Osama
Re: [PATCH] tty: n_tty: fix KCSAN data-race in n_tty_flush_buffer / n_tty_lookahead_flow_ctrl
Posted by Osama Abdelkader 1 month ago
On Wed, Feb 11, 2026 at 10:08:38PM +0100, Osama Abdelkader wrote:
> n_tty_lookahead_flow_ctrl() accesses ldata->lookahead_count without
> holding termios_rwsem, while reset_buffer_flags() in n_tty_flush_buffer()
> resets it with exclusive termios_rwsem held. This causes a data race
> reported by KCSAN when a PTY is closed while flush_to_ldisc is still
> processing lookahead data.
> 
> Fix by taking termios_rwsem (read) in n_tty_lookahead_flow_ctrl(),
> consistent with __receive_buf() which also modifies lookahead_count
> under the read lock.
> 
> Reported-by: syzbot+80806cf7508e92c7cc86@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=80806cf7508e92c7cc86
> Fixes: 6bb6fa6908eb ("tty: Implement lookahead to process XON/XOFF timely")
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
>  drivers/tty/n_tty.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index e6a0f5b40d0a..725d6ed2b863 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1480,6 +1480,8 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const u8 *cp,
>  	struct n_tty_data *ldata = tty->disc_data;
>  	u8 flag = TTY_NORMAL;
>  
> +	guard(rwsem_read)(&tty->termios_rwsem);
> +
>  	ldata->lookahead_count += count;
>  
>  	if (!I_IXON(tty))
> -- 
> 2.43.0
> 

ping