aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Johnston <markj@FreeBSD.org>2026-03-23 15:22:48 +0000
committerMark Johnston <markj@FreeBSD.org>2026-04-20 13:19:13 +0000
commitfdee312d0c97cf6d907eec0b96efd51bfc9a7567 (patch)
tree56dc867c3a3fc2110cde7d0f90f47394ddf89ece
parentf3bbb238daa17cad6a5a6d562686c2e10e9768e5 (diff)
tty: Avoid leaving dangling pointers in tty_drop_ctty()
The TIOCNOTTY handler detaches the calling process from its controlling terminal. It clears the link from the session to the tty, but not the pointers from the tty to the session and process group. This means that sess_release() doesn't call tty_rel_sess(), and that pgdelete() doesn't call tty_rel_pgrp(), so the pointers are left dangling. Fix this by clearing pointers in tty_drop_ctty(). Add a standalone regression test. Approved by: so Security: FreeBSD-SA-26:10.tty Security: CVE-2026-5398 Reported by: Nicholas Carlini <npc@anthropic.com> Reviewed by: kib, kevans Fixes: 1b50b999f9b5 ("tty: implement TIOCNOTTY") Differential Revision: https://reviews.freebsd.org/D56046
-rw-r--r--sys/kern/tty.c4
-rw-r--r--tests/sys/kern/tty/Makefile1
-rw-r--r--tests/sys/kern/tty/tiocnotty.c82
3 files changed, 87 insertions, 0 deletions
diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index c8e2c561b7cf..356a69682997 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -1261,6 +1261,10 @@ tty_drop_ctty(struct tty *tp, struct proc *p)
session->s_ttydp = NULL;
SESS_UNLOCK(session);
+ if (tp->t_session == session) {
+ tp->t_session = NULL;
+ tp->t_pgrp = NULL;
+ }
tp->t_sessioncnt--;
p->p_flag &= ~P_CONTROLT;
PROC_UNLOCK(p);
diff --git a/tests/sys/kern/tty/Makefile b/tests/sys/kern/tty/Makefile
index 8628ab79875f..d134b3337a21 100644
--- a/tests/sys/kern/tty/Makefile
+++ b/tests/sys/kern/tty/Makefile
@@ -5,6 +5,7 @@ PLAIN_TESTS_PORCH+= test_canon
PLAIN_TESTS_PORCH+= test_canon_fullbuf
PLAIN_TESTS_PORCH+= test_ncanon
PLAIN_TESTS_PORCH+= test_recanon
+PLAIN_TESTS_C+= tiocnotty
ATF_TESTS_C+= test_sti
PROGS+= fionread
diff --git a/tests/sys/kern/tty/tiocnotty.c b/tests/sys/kern/tty/tiocnotty.c
new file mode 100644
index 000000000000..2581f976b2ef
--- /dev/null
+++ b/tests/sys/kern/tty/tiocnotty.c
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2026 Mark Johnston <markj@FreeBSD.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+/*
+ * A regression test that exercises a bug where TIOCNOTTY would leave some
+ * dangling pointers behind in the controlling terminal structure.
+ */
+
+#include <sys/ioctl.h>
+#include <sys/wait.h>
+
+#include <err.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+int
+main(void)
+{
+ int master, slave, status;
+ pid_t child;
+
+ master = posix_openpt(O_RDWR | O_NOCTTY);
+ if (master < 0)
+ err(1, "posix_openpt");
+ if (grantpt(master) < 0)
+ err(1, "grantpt");
+ if (unlockpt(master) < 0)
+ err(1, "unlockpt");
+
+ child = fork();
+ if (child < 0)
+ err(1, "fork");
+ if (child == 0) {
+ if (setsid() < 0)
+ err(1, "setsid");
+ slave = open(ptsname(master), O_RDWR | O_NOCTTY);
+ if (slave < 0)
+ err(2, "open");
+ if (ioctl(slave, TIOCSCTTY, 0) < 0)
+ err(3, "ioctl(TIOCSCTTY)");
+ /* Detach ourselves from the controlling terminal. */
+ if (ioctl(slave, TIOCNOTTY, 0) < 0)
+ err(4, "ioctl(TIOCNOTTY)");
+ _exit(0);
+ }
+
+ if (waitpid(child, &status, 0) < 0)
+ err(1, "waitpid");
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+ errx(1, "child exited with status %d", WEXITSTATUS(status));
+
+ child = fork();
+ if (child < 0)
+ err(1, "fork");
+ if (child == 0) {
+ struct winsize winsz;
+
+ if (setsid() < 0)
+ err(1, "setsid");
+ slave = open(ptsname(master), O_RDWR | O_NOCTTY);
+ if (slave < 0)
+ err(2, "open");
+ /* Dereferences dangling t_pgrp pointer in the terminal. */
+ memset(&winsz, 0xff, sizeof(winsz));
+ if (ioctl(slave, TIOCSWINSZ, &winsz) < 0)
+ err(3, "ioctl(TIOCSWINSZ)");
+ /* Dereferences dangling t_session pointer in the terminal. */
+ if (ioctl(slave, TIOCSCTTY, 0) < 0)
+ err(4, "ioctl(TIOCSCTTY)");
+ _exit(0);
+ }
+
+ if (waitpid(child, &status, 0) < 0)
+ err(1, "waitpid");
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+ errx(1, "child exited with status %d", WEXITSTATUS(status));
+}