linux: Mark internal file descriptors as CLOEXEC
authorChris Dickens <christopher.a.dickens@gmail.com>
Mon, 20 Feb 2017 08:55:15 +0000 (00:55 -0800)
committerChris Dickens <christopher.a.dickens@gmail.com>
Wed, 1 Mar 2017 05:25:25 +0000 (21:25 -0800)
As a library, libusb should take care to be as friendly as possible
with various use cases. One such way is to ensure that internal file
descriptors have the CLOEXEC flag set, thus allowing processes to do
a fork() + exec() without leaking libusb's file descriptors to the
child process.

References #268

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
configure.ac
libusb/core.c
libusb/io.c
libusb/os/linux_netlink.c
libusb/os/linux_udev.c
libusb/os/linux_usbfs.c
libusb/os/poll_posix.c
libusb/version_nano.h

index c0347b9..272deea 100644 (file)
@@ -223,7 +223,7 @@ if test "x$use_timerfd" = xyes -a "x$timerfd_h" = x0; then
        AC_MSG_ERROR([timerfd header not available; glibc 2.9+ required])
 fi
 
-AC_CHECK_DECL([TFD_NONBLOCK], [tfd_hdr_ok=yes], [tfd_hdr_ok=no], [#include <sys/timerfd.h>])
+AC_CHECK_DECLS([TFD_NONBLOCK, TFD_CLOEXEC], [tfd_hdr_ok=yes], [tfd_hdr_ok=no], [#include <sys/timerfd.h>])
 if test "x$use_timerfd" = xyes -a "x$tfd_hdr_ok" = xno; then
        AC_MSG_ERROR([timerfd header not usable; glibc 2.9+ required])
 fi
@@ -240,6 +240,7 @@ else
        fi
 fi
 
+AC_CHECK_FUNCS([pipe2])
 AC_CHECK_TYPES([struct timespec])
 
 # Message logging
index d45bfe1..0f3f37e 100644 (file)
@@ -187,6 +187,20 @@ struct list_head active_contexts_list;
 /**
  * \page libusb_caveats Caveats
  *
+ * \section fork Fork considerations
+ *
+ * libusb is <em>not</em> designed to work across fork() calls. Depending on
+ * the platform, there may be resources in the parent process that are not
+ * available to the child (e.g. the hotplug monitor thread on Linux). In
+ * addition, since the parent and child will share libusb's internal file
+ * descriptors, using libusb in any way from the child could cause the parent
+ * process's \ref libusb_context to get into an inconsistent state.
+ *
+ * On Linux, libusb's file descriptors will be marked as CLOEXEC, which means
+ * that it is safe to fork() and exec() without worrying about the child
+ * process needing to clean up state or having access to these file descriptors.
+ * Other platforms may not be so forgiving, so consider yourself warned!
+ *
  * \section devresets Device resets
  *
  * The libusb_reset_device() function allows you to reset a device. If your
@@ -291,7 +305,6 @@ if (cfg != desired)
  * information about the end of the short packet, and the user probably wanted
  * that surplus data to arrive in the next logical transfer.
  *
- *
  * \section zlp Zero length packets
  *
  * - libusb is able to send a packet of zero length to an endpoint simply by
index eb1eabf..0bfb73d 100644 (file)
@@ -1145,7 +1145,7 @@ int usbi_io_init(struct libusb_context *ctx)
 
 #ifdef USBI_TIMERFD_AVAILABLE
        ctx->timerfd = timerfd_create(usbi_backend->get_timerfd_clockid(),
-               TFD_NONBLOCK);
+               TFD_NONBLOCK | TFD_CLOEXEC);
        if (ctx->timerfd >= 0) {
                usbi_dbg("using timerfd for timeouts");
                r = usbi_add_pollfd(ctx, ctx->timerfd, POLLIN);
index 2aadfa9..c1ad1ec 100644 (file)
 
 #define NL_GROUP_KERNEL 1
 
+#ifndef SOCK_CLOEXEC
+#define SOCK_CLOEXEC   0
+#endif
+
+#ifndef SOCK_NONBLOCK
+#define SOCK_NONBLOCK  0
+#endif
+
 static int linux_netlink_socket = -1;
 static int netlink_control_pipe[2] = { -1, -1 };
 static pthread_t libusb_linux_event_thread;
 
 static void *linux_netlink_event_thread_main(void *arg);
 
-static int set_fd_cloexec_nb(int fd)
+static int set_fd_cloexec_nb(int fd, int socktype)
 {
        int flags;
 
 #if defined(FD_CLOEXEC)
-       flags = fcntl(fd, F_GETFD);
-       if (flags == -1) {
-               usbi_err(NULL, "failed to get netlink fd flags (%d)", errno);
-               return -1;
-       }
+       /* Make sure the netlink socket file descriptor is marked as CLOEXEC */
+       if (!(socktype & SOCK_CLOEXEC)) {
+               flags = fcntl(fd, F_GETFD);
+               if (flags == -1) {
+                       usbi_err(NULL, "failed to get netlink fd flags (%d)", errno);
+                       return -1;
+               }
 
-       if (!(flags & FD_CLOEXEC)) {
                if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) {
                        usbi_err(NULL, "failed to set netlink fd flags (%d)", errno);
                        return -1;
@@ -70,13 +79,14 @@ static int set_fd_cloexec_nb(int fd)
        }
 #endif
 
-       flags = fcntl(fd, F_GETFL);
-       if (flags == -1) {
-               usbi_err(NULL, "failed to get netlink fd status flags (%d)", errno);
-               return -1;
-       }
+       /* Make sure the netlink socket is non-blocking */
+       if (!(socktype & SOCK_NONBLOCK)) {
+               flags = fcntl(fd, F_GETFL);
+               if (flags == -1) {
+                       usbi_err(NULL, "failed to get netlink fd status flags (%d)", errno);
+                       return -1;
+               }
 
-       if (!(flags & O_NONBLOCK)) {
                if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) {
                        usbi_err(NULL, "failed to set netlink fd status flags (%d)", errno);
                        return -1;
@@ -89,21 +99,15 @@ static int set_fd_cloexec_nb(int fd)
 int linux_netlink_start_event_monitor(void)
 {
        struct sockaddr_nl sa_nl = { .nl_family = AF_NETLINK, .nl_groups = NL_GROUP_KERNEL };
-       int socktype = SOCK_RAW;
+       int socktype = SOCK_RAW | SOCK_NONBLOCK | SOCK_CLOEXEC;
        int opt = 1;
        int ret;
 
-#if defined(SOCK_CLOEXEC)
-       socktype |= SOCK_CLOEXEC;
-#endif
-#if defined(SOCK_NONBLOCK)
-       socktype |= SOCK_NONBLOCK;
-#endif
-
        linux_netlink_socket = socket(PF_NETLINK, socktype, NETLINK_KOBJECT_UEVENT);
        if (linux_netlink_socket == -1 && errno == EINVAL) {
                usbi_dbg("failed to create netlink socket of type %d, attempting SOCK_RAW", socktype);
-               linux_netlink_socket = socket(PF_NETLINK, SOCK_RAW, NETLINK_KOBJECT_UEVENT);
+               socktype = SOCK_RAW;
+               linux_netlink_socket = socket(PF_NETLINK, socktype, NETLINK_KOBJECT_UEVENT);
        }
 
        if (linux_netlink_socket == -1) {
@@ -111,7 +115,7 @@ int linux_netlink_start_event_monitor(void)
                goto err;
        }
 
-       ret = set_fd_cloexec_nb(linux_netlink_socket);
+       ret = set_fd_cloexec_nb(linux_netlink_socket, socktype);
        if (ret == -1)
                goto err_close_socket;
 
index 3ba352d..67d7e39 100644 (file)
@@ -82,17 +82,33 @@ int linux_udev_start_event_monitor(void)
 
        udev_monitor_fd = udev_monitor_get_fd(udev_monitor);
 
+#if defined(FD_CLOEXEC)
+       /* Make sure the udev file descriptor is marked as CLOEXEC */
+       r = fcntl(udev_monitor_fd, F_GETFD);
+       if (r == -1) {
+               usbi_err(NULL, "geting udev monitor fd flags (%d)", errno);
+               goto err_free_monitor;
+       }
+       if (!(r & FD_CLOEXEC)) {
+               if (fcntl(udev_monitor_fd, F_SETFD, r | FD_CLOEXEC) == -1) {
+                       usbi_err(NULL, "setting udev monitor fd flags (%d)", errno);
+                       goto err_free_monitor;
+               }
+       }
+#endif
+
        /* Some older versions of udev are not non-blocking by default,
         * so make sure this is set */
        r = fcntl(udev_monitor_fd, F_GETFL);
        if (r == -1) {
-               usbi_err(NULL, "getting udev monitor fd flags (%d)", errno);
+               usbi_err(NULL, "getting udev monitor fd status flags (%d)", errno);
                goto err_free_monitor;
        }
-       r = fcntl(udev_monitor_fd, F_SETFL, r | O_NONBLOCK);
-       if (r) {
-               usbi_err(NULL, "setting udev monitor fd flags (%d)", errno);
-               goto err_free_monitor;
+       if (!(r & O_NONBLOCK)) {
+               if (fcntl(udev_monitor_fd, F_SETFL, r | O_NONBLOCK) == -1) {
+                       usbi_err(NULL, "setting udev monitor fd status flags (%d)", errno);
+                       goto err_free_monitor;
+               }
        }
 
        r = usbi_pipe(udev_control_pipe);
index 6b89ba2..ba437bd 100644 (file)
@@ -194,7 +194,7 @@ static int _get_usbfs_fd(struct libusb_device *dev, mode_t mode, int silent)
                snprintf(path, PATH_MAX, "%s/%03d/%03d",
                        usbfs_path, dev->bus_number, dev->device_address);
 
-       fd = open(path, mode);
+       fd = open(path, mode | O_CLOEXEC);
        if (fd != -1)
                return fd; /* Success */
 
@@ -205,7 +205,7 @@ static int _get_usbfs_fd(struct libusb_device *dev, mode_t mode, int silent)
                /* Wait 10ms for USB device path creation.*/
                nanosleep(&(struct timespec){delay / 1000000, (delay * 1000) % 1000000000UL}, NULL);
 
-               fd = open(path, mode);
+               fd = open(path, mode | O_CLOEXEC);
                if (fd != -1)
                        return fd; /* Success */
        }
@@ -526,7 +526,7 @@ static int _open_sysfs_attr(struct libusb_device *dev, const char *attr)
 
        snprintf(filename, PATH_MAX, "%s/%s/%s",
                SYSFS_DEVICE_PATH, priv->sysfs_dir, attr);
-       fd = open(filename, O_RDONLY);
+       fd = open(filename, O_RDONLY | O_CLOEXEC);
        if (fd < 0) {
                usbi_err(DEVICE_CTX(dev),
                        "open %s failed ret=%d errno=%d", filename, fd, errno);
@@ -546,7 +546,7 @@ static int __read_sysfs_attr(struct libusb_context *ctx,
 
        snprintf(filename, PATH_MAX, "%s/%s/%s", SYSFS_DEVICE_PATH,
                 devname, attr);
-       f = fopen(filename, "r");
+       f = fopen(filename, "re");
        if (f == NULL) {
                if (errno == ENOENT) {
                        /* File doesn't exist. Assume the device has been
index e2f55a5..337714a 100644 (file)
 
 int usbi_pipe(int pipefd[2])
 {
+#if defined(HAVE_PIPE2)
+       int ret = pipe2(pipefd, O_CLOEXEC);
+#else
        int ret = pipe(pipefd);
+#endif
+
        if (ret != 0) {
+               usbi_err(NULL, "failed to create pipe (%d)", errno);
                return ret;
        }
+
+#if !defined(HAVE_PIPE2) && defined(FD_CLOEXEC)
+       ret = fcntl(pipefd[0], F_GETFD);
+       if (ret == -1) {
+               usbi_err(NULL, "failed to get pipe fd flags (%d)", errno);
+               goto err_close_pipe;
+       }
+       ret = fcntl(pipefd[0], F_SETFD, ret | FD_CLOEXEC);
+       if (ret == -1) {
+               usbi_err(NULL, "failed to set pipe fd flags (%d)", errno);
+               goto err_close_pipe;
+       }
+
+       ret = fcntl(pipefd[1], F_GETFD);
+       if (ret == -1) {
+               usbi_err(NULL, "failed to get pipe fd flags (%d)", errno);
+               goto err_close_pipe;
+       }
+       ret = fcntl(pipefd[1], F_SETFD, ret | FD_CLOEXEC);
+       if (ret == -1) {
+               usbi_err(NULL, "failed to set pipe fd flags (%d)", errno);
+               goto err_close_pipe;
+       }
+#endif
+
        ret = fcntl(pipefd[1], F_GETFL);
        if (ret == -1) {
-               usbi_dbg("Failed to get pipe fd flags: %d", errno);
+               usbi_err(NULL, "failed to get pipe fd status flags (%d)", errno);
                goto err_close_pipe;
        }
        ret = fcntl(pipefd[1], F_SETFL, ret | O_NONBLOCK);
-       if (ret != 0) {
-               usbi_dbg("Failed to set non-blocking on new pipe: %d", errno);
+       if (ret == -1) {
+               usbi_err(NULL, "failed to set pipe fd status flags (%d)", errno);
                goto err_close_pipe;
        }
 
        return 0;
 
 err_close_pipe:
-       usbi_close(pipefd[0]);
-       usbi_close(pipefd[1]);
+       close(pipefd[0]);
+       close(pipefd[1]);
        return ret;
 }
index a7b48f3..6b6974f 100644 (file)
@@ -1 +1 @@
-#define LIBUSB_NANO 11185
+#define LIBUSB_NANO 11186