* [PATCH] tty: n_tty: order lockless input availability checks
@ 2026-05-04 7:23 Cen Zhang
2026-05-04 7:34 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Cen Zhang @ 2026-05-04 7:23 UTC (permalink / raw)
To: gregkh, jirislaby
Cc: peter, linux-kernel, linux-serial, baijiaju1990, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] tty: n_tty: order lockless input availability checks
2026-05-04 7:23 [PATCH] tty: n_tty: order lockless input availability checks Cen Zhang
@ 2026-05-04 7:34 ` Greg KH
2026-05-04 7:47 ` Cen Zhang
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2026-05-04 7:34 UTC (permalink / raw)
To: Cen Zhang; +Cc: jirislaby, peter, linux-kernel, linux-serial, baijiaju1990
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] tty: n_tty: order lockless input availability checks
2026-05-04 7:34 ` Greg KH
@ 2026-05-04 7:47 ` Cen Zhang
2026-05-04 7:53 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Cen Zhang @ 2026-05-04 7:47 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, peter, linux-kernel, linux-serial, baijiaju1990
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] tty: n_tty: order lockless input availability checks
2026-05-04 7:47 ` Cen Zhang
@ 2026-05-04 7:53 ` Greg KH
2026-05-04 8:11 ` Cen Zhang
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2026-05-04 7:53 UTC (permalink / raw)
To: Cen Zhang; +Cc: jirislaby, peter, linux-kernel, linux-serial, baijiaju1990
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tty: n_tty: order lockless input availability checks
2026-05-04 7:53 ` Greg KH
@ 2026-05-04 8:11 ` Cen Zhang
0 siblings, 0 replies; 5+ messages in thread
From: Cen Zhang @ 2026-05-04 8:11 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, peter, linux-kernel, linux-serial, baijiaju1990
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-04 8:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 7:23 [PATCH] tty: n_tty: order lockless input availability checks Cen Zhang
2026-05-04 7:34 ` Greg KH
2026-05-04 7:47 ` Cen Zhang
2026-05-04 7:53 ` Greg KH
2026-05-04 8:11 ` Cen Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox