[PATCH] tty: n_tty: order lockless input availability checks

[PATCH] tty: n_tty: order lockless input availability checks

[Cen Zhang]

The N_TTY read buffer uses release/acquire ordering for its
lockless ring indices. Input producers release-publish canon_head and
commit_head after updating the buffer and delimiter flags, and readers
acquire those heads before copying data. Readers also release-publish
read_tail before producers use it to calculate room.

chars_in_buffer() and input_available_p() sample the same indices
for availability and flow-control decisions, but use plain loads. That
can miss the ordering used by the data-copy paths and can also let
poll() observe termios-synthesized availability with weaker ordering
than normal receive-side publication.

Use acquire loads for the lockless head/tail samples in those
helpers. When n_tty_set_termios() updates canonical/noncanonical
availability, publish the updated heads with release stores as well.
Keep the cached icanon bit as an intentionally lockless mode snapshot
and annotate that access.

Fixes: 70aca71f92ca ("n_tty: Fix unordered accesses to lockless read buffer")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 drivers/tty/n_tty.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e6a0f5b40d0a..56b0cd96a453 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -213,9 +213,17 @@ static void n_tty_kick_worker(const struct tty_struct *tty)
 static ssize_t chars_in_buffer(const struct tty_struct *tty)
 {
 	const struct n_tty_data *ldata = tty->disc_data;
-	size_t head = ldata->icanon ? ldata->canon_head : ldata->commit_head;
+	bool icanon = data_race((int)ldata->icanon); /* lockless snapshot */
+	size_t head;
+	size_t tail;
 
-	return head - ldata->read_tail;
+	if (icanon)
+		head = smp_load_acquire(&ldata->canon_head); /* producer publish */
+	else
+		head = smp_load_acquire(&ldata->commit_head); /* producer publish */
+	tail = smp_load_acquire(&ldata->read_tail); /* consumer publish */
+
+	return head - tail;
 }
 
 /**
@@ -1779,14 +1787,14 @@ static void n_tty_set_termios(struct tty_struct *tty, const struct ktermios *old
 		bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
 		ldata->line_start = ldata->read_tail;
 		if (!L_ICANON(tty) || !read_cnt(ldata)) {
-			ldata->canon_head = ldata->read_tail;
+			smp_store_release(&ldata->canon_head, ldata->read_tail); /* publish */
 			ldata->push = 0;
 		} else {
 			set_bit(MASK(ldata->read_head - 1), ldata->read_flags);
-			ldata->canon_head = ldata->read_head;
+			smp_store_release(&ldata->canon_head, ldata->read_head); /* publish */
 			ldata->push = 1;
 		}
-		ldata->commit_head = ldata->read_head;
+		smp_store_release(&ldata->commit_head, ldata->read_head); /* publish */
 		ldata->erasing = 0;
 		ldata->lnext = 0;
 	}
@@ -1908,11 +1916,17 @@ 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;
+	bool icanon = data_race((int)ldata->icanon); /* lockless snapshot */
+	size_t tail = smp_load_acquire(&ldata->read_tail); /* consumer publish */
+	size_t head;
 
-	if (ldata->icanon && !L_EXTPROC(tty))
-		return ldata->canon_head != ldata->read_tail;
-	else
-		return ldata->commit_head - ldata->read_tail >= amt;
+	if (icanon && !L_EXTPROC(tty)) {
+		head = smp_load_acquire(&ldata->canon_head); /* producer publish */
+		return head != tail;
+	}
+
+	head = smp_load_acquire(&ldata->commit_head); /* producer publish */
+	return head - tail >= amt;
 }
 
 /**
-- 
2.43.0

Re: [PATCH] tty: n_tty: order lockless input availability checks

[Greg KH]

On Mon, May 04, 2026 at 03:23:21PM +0800, Cen Zhang wrote:
> The N_TTY read buffer uses release/acquire ordering for its
> lockless ring indices. Input producers release-publish canon_head and
> commit_head after updating the buffer and delimiter flags, and readers
> acquire those heads before copying data. Readers also release-publish
> read_tail before producers use it to calculate room.
> 
> chars_in_buffer() and input_available_p() sample the same indices
> for availability and flow-control decisions, but use plain loads. That
> can miss the ordering used by the data-copy paths and can also let
> poll() observe termios-synthesized availability with weaker ordering
> than normal receive-side publication.
> 
> Use acquire loads for the lockless head/tail samples in those
> helpers. When n_tty_set_termios() updates canonical/noncanonical
> availability, publish the updated heads with release stores as well.
> Keep the cached icanon bit as an intentionally lockless mode snapshot
> and annotate that access.
> 
> Fixes: 70aca71f92ca ("n_tty: Fix unordered accesses to lockless read buffer")
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

What tests show that this is needed?  That commit was a long time ago,
and surely we would have had some bug reports since then, right?

thanks,

greg k-h

Re: [PATCH] tty: n_tty: order lockless input availability checks

[Cen Zhang]

Dear Greg KH

Thanks for taking a look, and sorry if the changelog made this sound
stronger than the evidence I have.

> What tests show that this is needed?  That commit was a long time ago,
> and surely we would have had some bug reports since then, right?
>
> thanks,
>
> greg k-h

The evidence I have is from data-race reports produced while stressing
pty/tty ioctls.  The relevant stacks, mapped to current v7.0.3 source,
are:

  - read/unthrottle side:
      chars_in_buffer() at drivers/tty/n_tty.c:216
      n_tty_check_unthrottle() at drivers/tty/n_tty.c:275
      n_tty_read()

    racing with the receive side:
      n_tty_receive_char_canon() at drivers/tty/n_tty.c:1258
      __receive_buf() at drivers/tty/n_tty.c:1588

  - poll/select side:
      input_available_p() at drivers/tty/n_tty.c:1912-1915
      n_tty_poll() at drivers/tty/n_tty.c:2440/2444

    racing with termios changes:
      n_tty_set_termios() at drivers/tty/n_tty.c:1782,
      drivers/tty/n_tty.c:1786 and drivers/tty/n_tty.c:1789

My reasoning was that these helpers sample the same lockless read-buffer
availability state that the read/copy paths already handle with
smp_store_release()/smp_load_acquire(), but I do not have a test showing
a concrete functional failure beyond the data-race reports.

Would you prefer that I drop the Fixes tag and respin the changelog to
describe this as a conservative KCSAN/LKMM cleanup?  Or do you think the
evidence is too weak for a change here?

Thanks,
Cen

Re: [PATCH] tty: n_tty: order lockless input availability checks

[Greg KH]

On Mon, May 04, 2026 at 03:47:01PM +0800, Cen Zhang wrote:
> Dear Greg KH
> 
> Thanks for taking a look, and sorry if the changelog made this sound
> stronger than the evidence I have.
> 
> > What tests show that this is needed?  That commit was a long time ago,
> > and surely we would have had some bug reports since then, right?
> >
> > thanks,
> >
> > greg k-h
> 
> The evidence I have is from data-race reports produced while stressing
> pty/tty ioctls.  The relevant stacks, mapped to current v7.0.3 source,
> are:

Cool, where are those reports?

>   - read/unthrottle side:
>       chars_in_buffer() at drivers/tty/n_tty.c:216
>       n_tty_check_unthrottle() at drivers/tty/n_tty.c:275
>       n_tty_read()
> 
>     racing with the receive side:
>       n_tty_receive_char_canon() at drivers/tty/n_tty.c:1258
>       __receive_buf() at drivers/tty/n_tty.c:1588
> 
>   - poll/select side:
>       input_available_p() at drivers/tty/n_tty.c:1912-1915
>       n_tty_poll() at drivers/tty/n_tty.c:2440/2444
> 
>     racing with termios changes:
>       n_tty_set_termios() at drivers/tty/n_tty.c:1782,
>       drivers/tty/n_tty.c:1786 and drivers/tty/n_tty.c:1789
> 
> My reasoning was that these helpers sample the same lockless read-buffer
> availability state that the read/copy paths already handle with
> smp_store_release()/smp_load_acquire(), but I do not have a test showing
> a concrete functional failure beyond the data-race reports.
> 
> Would you prefer that I drop the Fixes tag and respin the changelog to
> describe this as a conservative KCSAN/LKMM cleanup?  Or do you think the
> evidence is too weak for a change here?

I don't really know as I haven't seen any such reports before that I can
recall.

thanks,

greg k-h

Re: [PATCH] tty: n_tty: order lockless input availability checks

[Cen Zhang]

Dear Greg KH

Thanks for taking a look.

> Cool, where are those reports?

Sorry, I should have been clearer.

These are local data-race reports from my pty/tty fuzzing run.
They were produced on v6.17-rc5:

  76eeb9b8de98 ("Linux 6.17-rc5")

The stack line numbers below are from that tested tree.  I also checked
current v7.0.3, and the same relevant plain lockless accesses are still
present there, although some surrounding line numbers have moved.

Report 1:

  ============ DATARACE ============

  Function: chars_in_buffer drivers/tty/n_tty.c:216 [inline]
  Function: n_tty_check_unthrottle+0x25c/0xbd0 drivers/tty/n_tty.c:275
  Function: tty_io_nonblock include/linux/tty.h:323 [inline]
  Function: n_tty_wait_for_input drivers/tty/n_tty.c:2163 [inline]
  Function: n_tty_read+0xed5/0x41f0 drivers/tty/n_tty.c:2264
  Function: tty_read+0x532/0xf50 drivers/tty/tty_io.c:904
  Function: new_sync_read fs/read_write.c:489 [inline]
  Function: vfs_read+0x5fe/0xb70 fs/read_write.c:572
  Function: ksys_read+0xf7/0x1e0 fs/read_write.c:712

  ============OTHER_INFO============

  Function: n_tty_receive_char_canon drivers/tty/n_tty.c:1259 [inline]
  Function: n_tty_receive_char_special drivers/tty/n_tty.c:1372 [inline]
  Function: n_tty_receive_buf_common+0x2cb0/0x3410 drivers/tty/n_tty.c:1588
  Function: n_tty_receive_buf2+0x51/0x80 drivers/tty/n_tty.c:1487
  Function: tty_flip_buffer_commit drivers/tty/tty_buffer.c:515 [inline]
  Function: tty_ldisc_receive_buf+0x1e8/0x450 drivers/tty/tty_buffer.c:532
  Function: paste_selection+0x781/0xcd0

Report 2:

  ============ DATARACE ============

  Function: input_available_p drivers/tty/n_tty.c:1926 [inline]
  Function: n_tty_poll+0x623/0x16b0 drivers/tty/n_tty.c:2452
  Function: tty_poll+0x224/0x4a0 drivers/tty/tty_io.c:2199
  Function: do_select+0xce7/0x13d0 fs/select.c:536
  Function: __do_sys_pselect6+0x1d8/0x240 fs/select.c:793

  ============OTHER_INFO============

  Function: n_tty_set_termios+0x82b/0x37a0 drivers/tty/n_tty.c:1799
  Function: tty_set_termios+0x112d/0x1b80 drivers/tty/tty_ioctl.c:348
  Function: set_termios+0xc1b/0xca0 drivers/tty/tty_ioctl.c:512
  Function: n_tty_ioctl_helper+0xe5/0x8f0 drivers/tty/tty_ioctl.c:982
  Function: n_tty_ioctl+0x253/0x730 drivers/tty/n_tty.c:2509
  Function: tty_ioctl+0x1cfb/0x3070 drivers/tty/tty_io.c:2801

In current v7.0.3, the same relevant source pattern is still present at:

  - chars_in_buffer():
      drivers/tty/n_tty.c:216-218

  - input_available_p():
      drivers/tty/n_tty.c:1912-1915

  - n_tty_set_termios():
      drivers/tty/n_tty.c:1782
      drivers/tty/n_tty.c:1786
      drivers/tty/n_tty.c:1789

Thanks,
Cen