Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 17 Aug 2023 13:49:10 GMT
From:      Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: a4aaee2120ce - main - forkpty: Avoid fd leak if fork() fails.
Message-ID:  <202308171349.37HDnAq7063884@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=a4aaee2120ce0a121f86e39e214c2fabe82f2762

commit a4aaee2120ce0a121f86e39e214c2fabe82f2762
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2023-08-17 13:48:42 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2023-08-17 13:48:42 +0000

    forkpty: Avoid fd leak if fork() fails.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    allanjude
    Differential Revision:  https://reviews.freebsd.org/D41491
---
 lib/libutil/pty.c                |  1 +
 lib/libutil/tests/Makefile       |  1 +
 lib/libutil/tests/forkpty_test.c | 58 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/lib/libutil/pty.c b/lib/libutil/pty.c
index f52407608e9a..e5b42a666c7f 100644
--- a/lib/libutil/pty.c
+++ b/lib/libutil/pty.c
@@ -95,6 +95,7 @@ forkpty(int *amaster, char *name, struct termios *termp, struct winsize *winp)
 		return (-1);
 	switch (pid = fork()) {
 	case -1:
+		(void)close(master);
 		(void)close(slave);
 		return (-1);
 	case 0:
diff --git a/lib/libutil/tests/Makefile b/lib/libutil/tests/Makefile
index 0c67747aeb3f..d29045d78a10 100644
--- a/lib/libutil/tests/Makefile
+++ b/lib/libutil/tests/Makefile
@@ -7,6 +7,7 @@ TAP_TESTS_C+=	trimdomain_test
 TAP_TESTS_C+=	trimdomain-nodomain_test
 ATF_TESTS_C+=	cpuset_test
 ATF_TESTS_C+=	expand_number_test
+ATF_TESTS_C+=	forkpty_test
 
 WARNS?=		2
 LIBADD+=	util
diff --git a/lib/libutil/tests/forkpty_test.c b/lib/libutil/tests/forkpty_test.c
new file mode 100644
index 000000000000..3e54cf310150
--- /dev/null
+++ b/lib/libutil/tests/forkpty_test.c
@@ -0,0 +1,58 @@
+/*-
+ * Copyright (c) 2023 Klara, Inc.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <sys/resource.h>
+#include <sys/wait.h>
+
+#include <errno.h>
+#include <libutil.h>
+#include <unistd.h>
+
+#include <atf-c.h>
+
+ATF_TC(forkfail);
+ATF_TC_HEAD(forkfail, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Check for fd leak when fork() fails");
+	atf_tc_set_md_var(tc, "require.user", "unprivileged");
+}
+
+ATF_TC_BODY(forkfail, tc)
+{
+	struct rlimit orl, nrl;
+	pid_t pid;
+	int prevfd, fd, pty;
+
+	/* set process limit to 1 so fork() will fail */
+	ATF_REQUIRE_EQ(0, getrlimit(RLIMIT_NPROC, &orl));
+	nrl = orl;
+	nrl.rlim_cur = 1;
+	ATF_REQUIRE_EQ(0, setrlimit(RLIMIT_NPROC, &nrl));
+	/* check first free fd */
+	ATF_REQUIRE((fd = dup(0)) > 0);
+	ATF_REQUIRE_EQ(0, close(fd));
+	/* attempt forkpty() */
+	pid = forkpty(&pty, NULL, NULL, NULL);
+	if (pid == 0) {
+		/* child - fork() unexpectedly succeeded */
+		_exit(0);
+	}
+	ATF_CHECK_ERRNO(EAGAIN, pid < 0);
+	if (pid > 0) {
+		/* parent - fork() unexpectedly succeeded */
+		(void)waitpid(pid, NULL, 0);
+	}
+	/* check that first free fd hasn't changed */
+	prevfd = fd;
+	ATF_REQUIRE((fd = dup(0)) > 0);
+	ATF_CHECK_EQ(prevfd, fd);
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+	ATF_TP_ADD_TC(tp, forkfail);
+	return (atf_no_error());
+}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202308171349.37HDnAq7063884>