write() calls on a tty without the IO_NDELAY flag are serialized inside ttydev_write(). Therefore, a thread writing to a tty without IO_NDELAY has exclusive access to that tty with respect to other writers not using IO_NDELAY. Let's assume a thread is writing to a tty without IO_NDELAY. It acquires exclusive access and calls ttydisc_write(). In ttydisc_write() it may sleep (interruptibly) on the &tp->t_outwait condition variable. It does _not_ relinquish its exclusive access before sleeping. If it receives a stop signal while it is sleeping it will additionally become suspended (see sig_suspend_threads()), so signaling the cv won't make it runnable anymore. The suspended thread still has exclusive access to the tty, preventing other threads from writing to it. There is a mechanism to defer handling of stop signals using sigdeferstop(), but I don't see it being used anywhere related to tty writes. Perhaps the call to ttydisc_write() could be put between sigdeferstop(SIGDEFERSTOP_ERESTART) and sigallowstop()?
Created attachment 224891 [details] ttydev_write: prevent stops while terminal is busied
Yes I think you analysis is correct, and suggested workaround is almost good. It is not safe to use ERESTART for writes, EINTR should be a better choice there.
(In reply to Konstantin Belousov from comment #2) Why is it not safe to use ERESTART? The syscall will be restarted only if no bytes were written from the uio buffer. If the write completed partially, dofilewrite() will return 0 and the syscall won't be restarted. In dofilewrite(): cnt = auio->uio_resid; if ((error = fo_write(fp, auio, td->td_ucred, flags, td))) { if (auio->uio_resid != cnt && (error == ERESTART || error == EINTR || error == EWOULDBLOCK)) error = 0; ... }
Created attachment 224907 [details] ttydev_write: prevent stops while terminal is busied
(In reply to Jakub Piecuch from comment #3) Ok, changed to ERESTART.
What's the way forward from here? Do we wait for someone else to review the patch? It looks good to me btw.
(In reply to Jakub Piecuch from comment #6) It sit waiting for your final agreement. I will commit it today or tomorrow.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8cf912b017a04a2eec01fbaa1f7b9ef556403ede commit 8cf912b017a04a2eec01fbaa1f7b9ef556403ede Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-05-13 01:35:06 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-05-18 17:52:03 +0000 ttydev_write: prevent stops while terminal is busied Since busy state is checked by all blocked writes, stopping a process which waits in ttydisc_write() causes cascade. Utilize sigdeferstop() to avoid the issue. Submitted by: Jakub Piecuch <j.piecuch96@gmail.com> PR: 255816 MFC after: 1 week sys/kern/tty.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=4abc8e0a7335e2e79e3bb03eaed660ddc9c72bba commit 4abc8e0a7335e2e79e3bb03eaed660ddc9c72bba Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-05-13 01:35:06 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-05-25 07:15:51 +0000 ttydev_write: prevent stops while terminal is busied PR: 255816 (cherry picked from commit 8cf912b017a04a2eec01fbaa1f7b9ef556403ede) sys/kern/tty.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=c66c9ae5dc4b11de990fd50627f62398ec88e8d1 commit c66c9ae5dc4b11de990fd50627f62398ec88e8d1 Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-05-13 01:35:06 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-05-25 07:32:13 +0000 ttydev_write: prevent stops while terminal is busied PR: 255816 (cherry picked from commit 8cf912b017a04a2eec01fbaa1f7b9ef556403ede) sys/kern/tty.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)