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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Mar 17, 2026 at 07:45:44PM +0800, ziyu zhang wrote:
> > Given that the previous patch was not even tested, how was this found
> > and most importantly, tested?
> 
> My apologies for sending the v1 patch without properly compile-testing
> it first.
> It is a data race found by our testing tool. The detailed report is
> showed below.

As per our documentation, you MUST document what type of tool you are
using when you find stuff like this, so we can push back and say "hey,
your tool is broken!"  :)

And as you never actually tested this, I'm kind of thinking that your
tool is broken...

> Kernel panic: ============ DATARACE ============
> VarName 11902078599461647536, BlockLineNumber 36, IrLineNumber 3, is write 1
> Function: found_watchpoint kernel/kccwf/wp_checker.c:126 [inline]
> Function: watchpoints_monitor+0x1ac2/0x22a0 kernel/kccwf/wp_checker.c:217
> Function: kccwf_rec_mem_access+0x275e/0x2b70
> Function: n_tty_set_termios+0x5d1/0x37a0 drivers/tty/n_tty.c:1793
> Function: tty_set_termios+0x112d/0x1b80 drivers/tty/tty_ioctl.c:348
> Function: set_termios+0xc1b/0xca0 drivers/tty/tty_ioctl.c:512
> Function: tty_mode_ioctl+0x5e5/0x9d0
> 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
> Function: vfs_ioctl fs/ioctl.c:51 [inline]
> Function: __do_sys_ioctl fs/ioctl.c:598 [inline]
> Function: __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
> Function: do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> Function: do_syscall_64+0xd2/0x200 arch/x86/entry/syscall_64.c:94
> Function: entry_SYSCALL_64_after_hwframe+0x77/0x7f
> Function: 0x0

What is panicing here?

> ============OTHER_INFO============
> VarName 16560329611981062523, BlockLineNumber 32, IrLineNumber 1,
> watchpoint index 38236
> Function: set_report_info+0xbd/0x2c0 kernel/kccwf/report.c:57
> Function: setup_watchpoint kernel/kccwf/wp_checker.c:158 [inline]
> Function: watchpoints_monitor+0xe0e/0x22a0 kernel/kccwf/wp_checker.c:239
> Function: kccwf_rec_mem_access+0x275e/0x2b70
> Function: input_available_p drivers/tty/n_tty.c:1923 [inline]
> Function: n_tty_poll+0x3f5/0x16b0 drivers/tty/n_tty.c:2452
> Function: tty_poll+0x224/0x4a0 drivers/tty/tty_io.c:2199
> Function: vfs_poll include/linux/poll.h:82 [inline]
> Function: select_poll_one fs/select.c:480 [inline]
> Function: do_select+0xce7/0x13d0 fs/select.c:536
> Function: core_sys_select+0x3e8/0x5d0 fs/select.c:677
> Function: do_pselect fs/select.c:759 [inline]
> Function: __do_sys_pselect6 fs/select.c:802 [inline]
> Function: __se_sys_pselect6+0x1d8/0x240 fs/select.c:793
> Function: do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> Function: do_syscall_64+0xd2/0x200 arch/x86/entry/syscall_64.c:94
> Function: entry_SYSCALL_64_after_hwframe+0x77/0x7f
> =================END==============
> 
> > What error was reported that this now
> > fixes?
> 
> Error: cannot pass bit-field as __auto_type initializer in C.
> The detailed error is showed below.
> 
> drivers/tty/n_tty.c:1913:6: error: cannot pass bit-field as
> __auto_type initializer in C
>     1913 |         if (data_race(ldata->icanon) && !L_EXTPROC(tty))
>          |             ^
>    include/linux/compiler.h:194:13: note: expanded from macro 'data_race'
>      194 |         auto __v = (expr);
>             \
>          |                    ^
>    1 error generated.

That is the error that the first version of your patch generated when
you actually built it, which proves you never actually tested it!

Always test your changes, ESPECIALLY for core portions of the kernel
that everyone relies on.

> > What changed to cause this to now be needed?
> 
> In my initial v1 patch, I carelessly wrapped the bit-field directly in
> the data_race() macro without locally compile-testing it.
> The data_race(expr) macro internally utilizes __auto_type (or typeof).
> And the C standard strictly prohibits passing a bit-field directly as
> an initializer for __auto_type.
> Adding !! forces the bit-field to be evaluated as a standard
> boolean/integer expression, which bypasses this compiler restriction.
> I sincerely apologize for the oversight in my initial patch and any
> extra work it may have caused.
> Thank you for your time and patience. I will submit a v3 patch if necessary.

Your v1 patch was wrong, and not tested, and I'm asking what changed to
require any of this?  What is wrong with the original code, and what
changed to cause it to suddenly be wrong as this is probably one of the
oldest parts of the kernel still in active use today.

thanks,

greg k-h