From aebaa32c3108e99cc28d9aef7229ed4fe7da18ca Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Mon, 15 Jan 2024 20:55:59 -0600 Subject: [PATCH] kern: pts: do not special case closed slave side This would previously return 1 if the slave side of the pts was closed to force an application to read() from it and observe the EOF, but it's not clear why and this is inconsistent both with how we handle devices with similar mechanics (like pipes) and also with other kernels, such as OpenBSD/NetBSD and Linux. PR: 239604 Reviewed by: kib (cherry picked from commit 30189156d325fbcc9d1997d791daedc9fa3bed20) --- sys/kern/tty_pts.c | 7 +---- tests/sys/kern/Makefile | 3 ++ tests/sys/kern/tty_pts.c | 64 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 tests/sys/kern/tty_pts.c diff --git a/sys/kern/tty_pts.c b/sys/kern/tty_pts.c index f79c9cc3e97..31122d422a2 100644 --- a/sys/kern/tty_pts.c +++ b/sys/kern/tty_pts.c @@ -271,12 +271,7 @@ ptsdev_ioctl(struct file *fp, u_long cmd, void *data, return (0); case FIONREAD: tty_lock(tp); - if (psc->pts_flags & PTS_FINISHED) { - /* Force read() to be called. */ - *(int *)data = 1; - } else { - *(int *)data = ttydisc_getc_poll(tp); - } + *(int *)data = ttydisc_getc_poll(tp); tty_unlock(tp); return (0); case FIODGNAME: diff --git a/tests/sys/kern/Makefile b/tests/sys/kern/Makefile index e449d9deeed..b95be12d966 100644 --- a/tests/sys/kern/Makefile +++ b/tests/sys/kern/Makefile @@ -38,6 +38,7 @@ ATF_TESTS_C+= subr_physmem_test PLAIN_TESTS_C+= subr_unit_test ATF_TESTS_C+= sysctl_kern_proc ATF_TESTS_C+= sys_getrandom +ATF_TESTS_C+= tty_pts ATF_TESTS_C+= unix_dgram ATF_TESTS_C+= unix_passfd_dgram TEST_METADATA.unix_passfd_dgram+= is_exclusive="true" @@ -86,6 +87,8 @@ NETBSD_ATF_TESTS_C+= sysv_test CFLAGS.mqueue_test+= -I${SRCTOP}/tests LIBADD.mqueue_test+= rt +LIBADD.tty_pts+= atf_c util + ATF_TESTS_C+= libkern_crc32 SRCS.libkern_crc32+= libkern_crc32.c .PATH: ${SRCTOP}/sys/libkern diff --git a/tests/sys/kern/tty_pts.c b/tests/sys/kern/tty_pts.c new file mode 100644 index 00000000000..241cb085a5a --- /dev/null +++ b/tests/sys/kern/tty_pts.c @@ -0,0 +1,64 @@ +/*- + * Copyright (c) 2024 Kyle Evans + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +#include + +#include +#include + +/* Just a little more concise. */ +#define newpty(masterp, slavep) openpty((masterp), (slavep), NULL, NULL, NULL) + +ATF_TC_WITHOUT_HEAD(fionread); +ATF_TC_BODY(fionread, tc) +{ + char rbuf[32]; + char buf[] = "Hello"; + int master, slave; + int bytes; + + ATF_REQUIRE_EQ(0, newpty(&master, &slave)); + + /* Should be empty to begin with. */ + ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes)); + ATF_REQUIRE_EQ(0, bytes); + + ATF_REQUIRE_EQ(sizeof(buf) - 1, write(slave, buf, sizeof(buf) - 1)); + ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes)); + ATF_REQUIRE_EQ(sizeof(buf) - 1, bytes); + + /* Drain what we have available, should result in 0 bytes again. */ + ATF_REQUIRE_EQ(sizeof(buf) - 1, read(master, rbuf, sizeof(rbuf))); + ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes)); + ATF_REQUIRE_EQ(0, bytes); + + /* + * Write once more, then close the slave side with data still in the + * buffer. + */ + ATF_REQUIRE_EQ(sizeof(buf) - 1, write(slave, buf, sizeof(buf) - 1)); + ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes)); + ATF_REQUIRE_EQ(sizeof(buf) - 1, bytes); + + ATF_REQUIRE_EQ(0, close(slave)); + + /* + * The tty's output queue is discarded upon close, so we shouldn't have + * anything else to read(). + */ + ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes)); + ATF_REQUIRE_EQ(0, bytes); + ATF_REQUIRE_EQ(0, read(master, rbuf, sizeof(rbuf))); +} + +ATF_TP_ADD_TCS(tp) +{ + ATF_TP_ADD_TC(tp, fionread); + return (atf_no_error()); +}