Author: markj
Date: Tue Nov 13 20:07:55 2018
New Revision: 340408
URL: https://svnweb.freebsd.org/changeset/base/340408

Log:
  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

Modified:
  head/lib/libnv/common_impl.h
  head/lib/libnv/msgio.c
  head/lib/libnv/tests/nvlist_send_recv_test.c
  head/sys/contrib/libnv/nvpair.c

Modified: head/lib/libnv/common_impl.h
==============================================================================
--- head/lib/libnv/common_impl.h        Tue Nov 13 19:53:02 2018        
(r340407)
+++ head/lib/libnv/common_impl.h        Tue Nov 13 20:07:55 2018        
(r340408)
@@ -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_ */

Modified: head/lib/libnv/msgio.c
==============================================================================
--- head/lib/libnv/msgio.c      Tue Nov 13 19:53:02 2018        (r340407)
+++ head/lib/libnv/msgio.c      Tue Nov 13 20:07:55 2018        (r340408)
@@ -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));

Modified: head/lib/libnv/tests/nvlist_send_recv_test.c
==============================================================================
--- head/lib/libnv/tests/nvlist_send_recv_test.c        Tue Nov 13 19:53:02 
2018        (r340407)
+++ head/lib/libnv/tests/nvlist_send_recv_test.c        Tue Nov 13 20:07:55 
2018        (r340408)
@@ -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);
 }

Modified: head/sys/contrib/libnv/nvpair.c
==============================================================================
--- head/sys/contrib/libnv/nvpair.c     Tue Nov 13 19:53:02 2018        
(r340407)
+++ head/sys/contrib/libnv/nvpair.c     Tue Nov 13 20:07:55 2018        
(r340408)
@@ -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
                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 in
 
        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);
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to