tty: n_hdlc: fix hangup handling

An LLM incorrectly flagged three hangup issues in n_hdlc as leading to hangups stuck in uninterruptible sleep.

This used to be the case before commit c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") which incidentally fixed the stuck hangup issues by turning tty_io_nonblock() into an indirect hangup check.

Since then read() and write() can return -EAGAIN also for files opened in blocking mode on ldisc changes, which looks questionable. This change was added to address a regression introduced by a series adding missing locking to tty_reopen() but should probably be reconsidered. [1]

This series addresses the underlying hangup issues and incorrect -EAGAIN on hangup meanwhile.

Johan

[1] https://lore.kernel.org/all/alpine.LRH.2.02.1809191330590.19146@file01.intranet.prod.int.rdu2.redhat.com/

Johan Hovold (3):
tty: n_hdlc: fix missing write() hangup check
tty: n_hdlc: fix read() hangup race
tty: n_hdlc: fix read() hangup check

 drivers/tty/n_hdlc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

2.55.0

Add the missing hangup check to the n_hdlc write() wait loop so that hangups are detected and -EIO is returned as expected.

Note that before commit c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") a task hanging up the tty would get stuck in uninterruptible sleep when there are blocked writers, but since then write() incorrectly returns -EAGAIN instead.

The false-positive stuck hangup issue was flagged by an LLM.

---
 drivers/tty/n_hdlc.c | 5 +++++
 1 file changed, 5 insertions(+)

index 895b850a5950..0fbc44968a44 100644
@@ -529,6 +529,11 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
 	for (;;) {
 		set_current_state(TASK_INTERRUPTIBLE);
 
+		if (tty_hung_up_p(file)) {
+			error = -EIO;
+			break;
+		}
+
 		tbuf = n_hdlc_buf_get(&n_hdlc->tx_free_buf_list);
 		if (tbuf)
 			break;

2.55.0

Make sure to set the task state before checking the break conditions in the read() wait loop to avoid missing a racing wakeup.

Note that before commit c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") a racing task hanging up the tty could get stuck in uninterruptible sleep when there are blocked readers, but since then read() incorrectly returns -EAGAIN instead.

The false-positive stuck hangup issue was flagged by an LLM.

---
 drivers/tty/n_hdlc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 0fbc44968a44..02676ae9b112 100644
@@ -431,6 +431,8 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 	add_wait_queue(&tty->read_wait, &wait);
 
 	for (;;) {
+		set_current_state(TASK_INTERRUPTIBLE);
+
 		if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
 			ret = -EIO;
 			break;

@@ -438,8 +440,6 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 		if (tty_hung_up_p(file))
 			break;
 
-		set_current_state(TASK_INTERRUPTIBLE);
-
 		rbuf = n_hdlc_buf_get(&n_hdlc->rx_buf_list);
 		if (rbuf)
 			break;

2.55.0

When a tty is hung up, the file ops are generally set to hung_up_tty_fops, but this is not the case for, for example, /dev/console.

Make sure to check the TTY_HUPPING flag so that a hangup is detected and read() returns zero also with /dev/console (as is done for n_tty [1]).

Note that before commit c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") a task hanging up the tty would get stuck in uninterruptible sleep when there are blocked readers, but since then read() incorrectly returns -EAGAIN instead.

The false-positive stuck hangup issue was flagged by an LLM.

[1] 28b0f8a6962a ("tty: make n_tty_read() always abort if hangup is in progress")

---
 drivers/tty/n_hdlc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 02676ae9b112..1a3a276f2db3 100644
@@ -437,7 +437,8 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 			ret = -EIO;
 			break;
 		}
-		if (tty_hung_up_p(file))
+
+		if (tty_hung_up_p(file) || test_bit(TTY_HUPPING, &tty->flags))
 			break;
 
 		rbuf = n_hdlc_buf_get(&n_hdlc->rx_buf_list);

2.55.0