From aa73b2ecb81f1fe4f503d3ac693679632587ad50 Mon Sep 17 00:00:00 2001 From: Chris Dickens Date: Mon, 20 Feb 2017 00:55:15 -0800 Subject: [PATCH] linux: Mark internal file descriptors as CLOEXEC 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 --- configure.ac | 3 ++- libusb/core.c | 15 +++++++++++- libusb/io.c | 2 +- libusb/os/linux_netlink.c | 50 +++++++++++++++++++++------------------ libusb/os/linux_udev.c | 26 ++++++++++++++++---- libusb/os/linux_usbfs.c | 8 +++---- libusb/os/poll_posix.c | 41 ++++++++++++++++++++++++++++---- libusb/version_nano.h | 2 +- 8 files changed, 106 insertions(+), 41 deletions(-) diff --git a/configure.ac b/configure.ac index c0347b9..272deea 100644 --- a/configure.ac +++ b/configure.ac @@ -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 ]) +AC_CHECK_DECLS([TFD_NONBLOCK, TFD_CLOEXEC], [tfd_hdr_ok=yes], [tfd_hdr_ok=no], [#include ]) 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 diff --git a/libusb/core.c b/libusb/core.c index d45bfe1..0f3f37e 100644 --- a/libusb/core.c +++ b/libusb/core.c @@ -187,6 +187,20 @@ struct list_head active_contexts_list; /** * \page libusb_caveats Caveats * + * \section fork Fork considerations + * + * libusb is not 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 diff --git a/libusb/io.c b/libusb/io.c index eb1eabf..0bfb73d 100644 --- a/libusb/io.c +++ b/libusb/io.c @@ -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); diff --git a/libusb/os/linux_netlink.c b/libusb/os/linux_netlink.c index 2aadfa9..c1ad1ec 100644 --- a/libusb/os/linux_netlink.c +++ b/libusb/os/linux_netlink.c @@ -45,24 +45,33 @@ #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; diff --git a/libusb/os/linux_udev.c b/libusb/os/linux_udev.c index 3ba352d..67d7e39 100644 --- a/libusb/os/linux_udev.c +++ b/libusb/os/linux_udev.c @@ -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); diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c index 6b89ba2..ba437bd 100644 --- a/libusb/os/linux_usbfs.c +++ b/libusb/os/linux_usbfs.c @@ -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 diff --git a/libusb/os/poll_posix.c b/libusb/os/poll_posix.c index e2f55a5..337714a 100644 --- a/libusb/os/poll_posix.c +++ b/libusb/os/poll_posix.c @@ -29,25 +29,56 @@ 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; } diff --git a/libusb/version_nano.h b/libusb/version_nano.h index a7b48f3..6b6974f 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11185 +#define LIBUSB_NANO 11186