From 991666adc717e4f5fdf9854f67d240a00f6eb4ab Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj@FreeBSD.org>
Date: Tue, 13 Nov 2018 20:07:55 +0000
Subject: [PATCH] Ensure that libnv can be used when kern.trap_enotcap=1.

libnv used fcntl(fd, F_GETFL) to test whether fd is a valid file
descriptor.  Aside from being racy, this check requires CAP_FCNTL
rights on fd.  Instead, use fcntl(fd, F_GETFD), which does not require
any capability rights.

Also remove some redundant fd_is_valid() checks to avoid extra system
calls; in many cases we were performing this check immediately before
dup()ing the descriptor.

Reviewed by:	cem, oshogbo (previous version)
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D17963
---
 lib/libnv/common_impl.h                 | 11 ++++++-
 lib/libnv/msgio.c                       |  5 ----
 lib/libnv/tests/nvlist_send_recv_test.c | 38 +++++++++++++++++++++----
 sys/contrib/libnv/nvpair.c              | 14 ---------
 4 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/lib/libnv/common_impl.h b/lib/libnv/common_impl.h
index a50902d221ce..69154466203a 100644
--- a/lib/libnv/common_impl.h
+++ b/lib/libnv/common_impl.h
@@ -34,6 +34,15 @@
 #ifndef	_COMMON_IMPL_H_
 #define	_COMMON_IMPL_H_
 
-#define	fd_is_valid(fd)	(fcntl((fd), F_GETFL) != -1 || errno != EBADF)
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+
+static inline bool
+fd_is_valid(int fd)
+{
+
+	return (fcntl(fd, F_GETFD) != -1 || errno != EBADF);
+}
 
 #endif	/* !_COMMON_IMPL_H_ */
diff --git a/lib/libnv/msgio.c b/lib/libnv/msgio.c
index 300a437a7421..938bca81b0ac 100644
--- a/lib/libnv/msgio.c
+++ b/lib/libnv/msgio.c
@@ -66,11 +66,6 @@ msghdr_add_fd(struct cmsghdr *cmsg, int fd)
 
 	PJDLOG_ASSERT(fd >= 0);
 
-	if (!fd_is_valid(fd)) {
-		errno = EBADF;
-		return (-1);
-	}
-
 	cmsg->cmsg_level = SOL_SOCKET;
 	cmsg->cmsg_type = SCM_RIGHTS;
 	cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
diff --git a/lib/libnv/tests/nvlist_send_recv_test.c b/lib/libnv/tests/nvlist_send_recv_test.c
index f4bc1c4464bc..1b673b5d4259 100644
--- a/lib/libnv/tests/nvlist_send_recv_test.c
+++ b/lib/libnv/tests/nvlist_send_recv_test.c
@@ -306,15 +306,12 @@ parent(int sock)
 	CHECK(name == NULL);
 }
 
-int
-main(void)
+static void
+send_nvlist(void)
 {
 	int status, socks[2];
 	pid_t pid;
 
-	printf("1..134\n");
-	fflush(stdout);
-
 	if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) < 0)
 		err(1, "socketpair() failed");
 	pid = fork();
@@ -326,7 +323,7 @@ main(void)
 		/* Child. */
 		close(socks[0]);
 		child(socks[1]);
-		return (0);
+		_exit(0);
 	default:
 		/* Parent. */
 		close(socks[1]);
@@ -336,6 +333,35 @@ main(void)
 
 	if (waitpid(pid, &status, 0) < 0)
 		err(1, "waitpid() failed");
+}
+
+static void
+send_closed_fd(void)
+{
+	nvlist_t *nvl;
+	int error, socks[2];
+
+	if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) < 0)
+		err(1, "socketpair() failed");
+
+	nvl = nvlist_create(0);
+	nvlist_add_descriptor(nvl, "fd", 12345);
+	error = nvlist_error(nvl);
+	CHECK(error == EBADF);
+
+	error = nvlist_send(socks[1], nvl);
+	CHECK(error != 0 && errno == EBADF);
+}
+
+int
+main(void)
+{
+
+	printf("1..136\n");
+	fflush(stdout);
+
+	send_nvlist();
+	send_closed_fd();
 
 	return (0);
 }
diff --git a/sys/contrib/libnv/nvpair.c b/sys/contrib/libnv/nvpair.c
index fc2350d9f0db..23038c5c5b71 100644
--- a/sys/contrib/libnv/nvpair.c
+++ b/sys/contrib/libnv/nvpair.c
@@ -1276,11 +1276,6 @@ nvpair_create_descriptor(const char *name, int value)
 {
 	nvpair_t *nvp;
 
-	if (value < 0 || !fd_is_valid(value)) {
-		ERRNO_SET(EBADF);
-		return (NULL);
-	}
-
 	value = fcntl(value, F_DUPFD_CLOEXEC, 0);
 	if (value < 0)
 		return (NULL);
@@ -1517,11 +1512,6 @@ nvpair_create_descriptor_array(const char *name, const int *value,
 		if (value[ii] == -1) {
 			fds[ii] = -1;
 		} else {
-			if (!fd_is_valid(value[ii])) {
-				ERRNO_SET(EBADF);
-				goto fail;
-			}
-
 			fds[ii] = fcntl(value[ii], F_DUPFD_CLOEXEC, 0);
 			if (fds[ii] == -1)
 				goto fail;
@@ -2035,10 +2025,6 @@ nvpair_append_descriptor_array(nvpair_t *nvp, const int value)
 
 	NVPAIR_ASSERT(nvp);
 	PJDLOG_ASSERT(nvp->nvp_type == NV_TYPE_DESCRIPTOR_ARRAY);
-	if (value < 0 || !fd_is_valid(value)) {
-		ERRNO_SET(EBADF);
-		return -1;
-	}
 	fd = fcntl(value, F_DUPFD_CLOEXEC, 0);
 	if (fd == -1) {
 		return (-1);