From e3af0032b236dc52d4a74c4d740e57a1f6d520aa Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 7 Nov 2009 18:55:16 +0100 Subject: set the close-on-exec flag on all file descriptors Added the "fd_util" library, which attempts to use the new thread-safe Linux system calls pipe2(), accept4() and the options O_CLOEXEC, SOCK_CLOEXEC. Without these, it falls back to FD_CLOEXEC, which is not thread safe. This is particularly important for the "pipe" output plugin (and others, such as JACK/PulseAudio), because we were heavily leaking file descriptors to child processes. --- Makefile.am | 8 ++ NEWS | 1 + configure.ac | 4 +- src/event_pipe.c | 7 +- src/fd_util.c | 171 ++++++++++++++++++++++++++++++++++++ src/fd_util.h | 53 +++++++++++ src/input/file_input_plugin.c | 3 +- src/listen.c | 5 +- src/log.c | 3 +- src/mixer/oss_mixer_plugin.c | 3 +- src/output/fifo_output_plugin.c | 5 +- src/output/httpd_output_plugin.c | 5 +- src/output/mvp_plugin.c | 8 +- src/output/oss_plugin.c | 10 ++- src/output/recorder_output_plugin.c | 3 +- src/output/solaris_output_plugin.c | 3 +- src/socket_util.c | 3 +- 17 files changed, 271 insertions(+), 24 deletions(-) create mode 100644 src/fd_util.c create mode 100644 src/fd_util.h diff --git a/Makefile.am b/Makefile.am index 3ba6613f8..542049be2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -72,6 +72,7 @@ mpd_headers = \ src/encoder_list.h \ src/encoder_api.h \ src/exclude.h \ + src/fd_util.h \ src/fifo_buffer.h \ src/update.h \ src/update_internal.h \ @@ -220,6 +221,7 @@ src_mpd_SOURCES = \ src/database.c \ src/dirvec.c \ src/exclude.c \ + src/fd_util.c \ src/fifo_buffer.c \ src/filter_plugin.c \ src/filter_registry.c \ @@ -738,6 +740,7 @@ test_run_input_LDADD = $(MPD_LIBS) \ test_run_input_SOURCES = test/run_input.c \ src/conf.c src/tokenizer.c src/utils.c \ src/tag.c src/tag_pool.c src/tag_save.c \ + src/fd_util.c \ $(ARCHIVE_SRC) \ $(INPUT_SRC) @@ -753,6 +756,7 @@ test_dump_playlist_SOURCES = test/dump_playlist.c \ src/uri.c \ src/song.c src/tag.c src/tag_pool.c src/tag_save.c \ src/text_input_stream.c src/fifo_buffer.c \ + src/fd_util.c \ $(ARCHIVE_SRC) \ $(INPUT_SRC) \ $(PLAYLIST_SRC) @@ -771,6 +775,7 @@ test_run_decoder_SOURCES = test/run_decoder.c \ src/tag.c src/tag_pool.c \ src/replay_gain.c \ src/uri.c \ + src/fd_util.c \ $(ARCHIVE_SRC) \ $(INPUT_SRC) \ $(TAG_SRC) \ @@ -790,6 +795,7 @@ test_read_tags_SOURCES = test/read_tags.c \ src/tag.c src/tag_pool.c \ src/replay_gain.c \ src/uri.c \ + src/fd_util.c \ $(ARCHIVE_SRC) \ $(INPUT_SRC) \ $(TAG_SRC) \ @@ -857,6 +863,7 @@ test_run_output_SOURCES = test/run_output.c \ src/filter/convert_filter_plugin.c \ src/filter/volume_filter_plugin.c \ src/pcm_volume.c \ + src/fd_util.c \ $(OUTPUT_SRC) test_read_mixer_CPPFLAGS = $(AM_CPPFLAGS) \ @@ -869,6 +876,7 @@ test_read_mixer_SOURCES = test/read_mixer.c \ src/mixer_control.c src/mixer_api.c \ src/filter_plugin.c \ src/filter/volume_filter_plugin.c \ + src/fd_util.c \ $(MIXER_SRC) endif diff --git a/NEWS b/NEWS index 40e37822c..96fcfb1f7 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,7 @@ ver 0.16 (20??/??/??) * state_file: save only if something has changed * database: eliminated maximum line length * log: redirect stdout/stderr to /dev/null if syslog is used +* set the close-on-exec flag on all file descriptors * obey $(sysconfdir) for default mpd.conf location * build with large file support by default * require GLib 2.16 diff --git a/configure.ac b/configure.ac index 5f9b98111..fb2f6a916 100644 --- a/configure.ac +++ b/configure.ac @@ -115,12 +115,14 @@ fi AC_CHECK_LIB(socket,socket,MPD_LIBS="$MPD_LIBS -lsocket",) AC_CHECK_LIB(nsl,gethostbyname,MPD_LIBS="$MPD_LIBS -lnsl",) +AC_CHECK_FUNCS(pipe2 accept4) + AC_CHECK_LIB(m,exp,MPD_LIBS="$MPD_LIBS -lm",) AC_CHECK_HEADERS(locale.h) AC_CHECK_HEADERS(valgrind/memcheck.h) -AC_CHECK_FUNCS(inotify_init) +AC_CHECK_FUNCS(inotify_init inotify_init1) AC_ARG_ENABLE(inotify, AS_HELP_STRING([--disable-inotify], [disable support Inotify automatic database update (default: enabled) ]),, diff --git a/src/event_pipe.c b/src/event_pipe.c index 3e5009150..17e11097a 100644 --- a/src/event_pipe.c +++ b/src/event_pipe.c @@ -19,6 +19,7 @@ #include "event_pipe.h" #include "utils.h" +#include "fd_util.h" #include #include @@ -84,11 +85,7 @@ void event_pipe_init(void) GIOChannel *channel; int ret; -#ifdef WIN32 - ret = _pipe(event_pipe, 512, _O_BINARY); -#else - ret = pipe(event_pipe); -#endif + ret = pipe_cloexec(event_pipe); if (ret < 0) g_error("Couldn't open pipe: %s", strerror(errno)); #ifndef WIN32 diff --git a/src/fd_util.c b/src/fd_util.c new file mode 100644 index 000000000..b6593d919 --- /dev/null +++ b/src/fd_util.c @@ -0,0 +1,171 @@ +/* + * Copyright (C) 2003-2009 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "fd_util.h" +#include "config.h" + +#if !defined(_GNU_SOURCE) && (defined(HAVE_PIPE2) || defined(HAVE_ACCEPT4)) +#define _GNU_SOURCE +#endif + +#include +#include +#include +#include + +#ifdef WIN32 +#include +#else +#include +#endif + +#ifndef WIN32 + +static int +fd_mask_flags(int fd, int and_mask, int xor_mask) +{ + int ret; + + assert(fd >= 0); + + ret = fcntl(fd, F_GETFD, 0); + if (ret < 0) + return ret; + + return fcntl(fd, F_SETFD, (ret & and_mask) ^ xor_mask); +} + +#endif /* !WIN32 */ + +int +fd_set_cloexec(int fd, bool enable) +{ +#ifndef WIN32 + return fd_mask_flags(fd, ~FD_CLOEXEC, enable ? FD_CLOEXEC : 0); +#else + (void)fd; + (void)enable; +#endif +} + +int +open_cloexec(const char *path_fs, int flags) +{ + int fd; + +#ifdef O_CLOEXEC + flags |= O_CLOEXEC; +#endif + +#ifdef O_NOCTTY + flags |= O_NOCTTY; +#endif + + fd = open(path_fs, flags, 0666); + fd_set_cloexec(fd, true); + + return fd; +} + +int +creat_cloexec(const char *path_fs, int mode) +{ + int flags = O_CREAT|O_WRONLY|O_TRUNC; + int fd; + +#ifdef O_CLOEXEC + flags |= O_CLOEXEC; +#endif + +#ifdef O_NOCTTY + flags |= O_NOCTTY; +#endif + + fd = open(path_fs, flags, mode); + fd_set_cloexec(fd, true); + + return fd; +} + +int +pipe_cloexec(int fd[2]) +{ +#ifdef WIN32 + return _pipe(event_pipe, 512, _O_BINARY); +#else + int ret; + +#ifdef HAVE_PIPE2 + ret = pipe2(fd, O_CLOEXEC); + if (ret >= 0 || errno != ENOSYS) + return ret; +#endif + + ret = pipe(fd); + if (ret >= 0) { + fd_set_cloexec(fd[0], true); + fd_set_cloexec(fd[1], true); + } + + return ret; +#endif +} + +int +socket_cloexec(int domain, int type, int protocol) +{ + int fd; + +#ifdef SOCK_CLOEXEC + fd = socket(domain, type | SOCK_CLOEXEC, protocol); + if (fd >= 0 || errno != EINVAL) + return fd; +#endif + + fd = socket(domain, type, protocol); + if (fd >= 0) + fd_set_cloexec(fd, true); + + return fd; +} + +int +accept_cloexec(int fd, struct sockaddr *address, size_t *address_length_r) +{ + int ret; + socklen_t address_length = *address_length_r; + +#ifdef HAVE_ACCEPT4 + ret = accept4(fd, address, &address_length, SOCK_CLOEXEC); + if (ret >= 0 || errno != ENOSYS) { + if (ret >= 0) + *address_length_r = address_length; + + return ret; + } +#endif + + ret = accept(fd, address, &address_length); + if (ret >= 0) { + fd_set_cloexec(ret, true); + *address_length_r = address_length; + } + + return ret; +} diff --git a/src/fd_util.h b/src/fd_util.h new file mode 100644 index 000000000..805d1cc3d --- /dev/null +++ b/src/fd_util.h @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2003-2009 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/* + * This library provides easy helper functions for working with file + * descriptors. It has wrappers for taking advantage of Linux 2.6 + * specific features like O_CLOEXEC. + * + */ + +#ifndef FD_UTIL_H +#define FD_UTIL_H + +#include +#include + +struct sockaddr; + +int +fd_set_cloexec(int fd, bool enable); + +int +open_cloexec(const char *path_fs, int flags); + +int +creat_cloexec(const char *path_fs, int mode); + +int +pipe_cloexec(int fd[2]); + +int +socket_cloexec(int domain, int type, int protocol); + +int +accept_cloexec(int fd, struct sockaddr *address, size_t *address_length_r); + +#endif diff --git a/src/input/file_input_plugin.c b/src/input/file_input_plugin.c index 07c1e4ed6..55b495f38 100644 --- a/src/input/file_input_plugin.c +++ b/src/input/file_input_plugin.c @@ -20,6 +20,7 @@ #include "config.h" /* must be first for large file support */ #include "input/file_input_plugin.h" #include "input_plugin.h" +#include "fd_util.h" #include #include @@ -50,7 +51,7 @@ input_file_open(struct input_stream *is, const char *filename) *slash = '\0'; } - fd = open(pathname, O_RDONLY); + fd = open_cloexec(pathname, O_RDONLY); if (fd < 0) { is->error = errno; g_debug("Failed to open \"%s\": %s", diff --git a/src/listen.c b/src/listen.c index 4728e7c7a..2490780ad 100644 --- a/src/listen.c +++ b/src/listen.c @@ -22,6 +22,7 @@ #include "client.h" #include "conf.h" #include "utils.h" +#include "fd_util.h" #include "config.h" #include @@ -426,9 +427,9 @@ listen_in_event(G_GNUC_UNUSED GIOChannel *source, { int listen_fd = GPOINTER_TO_INT(data), fd; struct sockaddr_storage sa; - socklen_t sa_length = sizeof(sa); + size_t sa_length = sizeof(sa); - fd = accept(listen_fd, (struct sockaddr*)&sa, &sa_length); + fd = accept_cloexec(listen_fd, (struct sockaddr*)&sa, &sa_length); if (fd >= 0) { set_nonblocking(fd); diff --git a/src/log.c b/src/log.c index 3a9795d7d..b2de391bd 100644 --- a/src/log.c +++ b/src/log.c @@ -20,6 +20,7 @@ #include "log.h" #include "conf.h" #include "utils.h" +#include "fd_util.h" #include "config.h" #include @@ -128,7 +129,7 @@ open_log_file(void) { assert(out_filename != NULL); - return open(out_filename, O_CREAT | O_WRONLY | O_APPEND, 0666); + return open_cloexec(out_filename, O_CREAT | O_WRONLY | O_APPEND); } static void diff --git a/src/mixer/oss_mixer_plugin.c b/src/mixer/oss_mixer_plugin.c index 4e169bbc4..028786ae9 100644 --- a/src/mixer/oss_mixer_plugin.c +++ b/src/mixer/oss_mixer_plugin.c @@ -19,6 +19,7 @@ #include "mixer_api.h" #include "output_api.h" +#include "fd_util.h" #include @@ -122,7 +123,7 @@ oss_mixer_open(struct mixer *data, GError **error_r) { struct oss_mixer *om = (struct oss_mixer *) data; - om->device_fd = open(om->device, O_RDONLY); + om->device_fd = open_cloexec(om->device, O_RDONLY); if (om->device_fd < 0) { g_set_error(error_r, oss_mixer_quark(), errno, "failed to open %s: %s", diff --git a/src/output/fifo_output_plugin.c b/src/output/fifo_output_plugin.c index d3145748f..0217c2675 100644 --- a/src/output/fifo_output_plugin.c +++ b/src/output/fifo_output_plugin.c @@ -20,6 +20,7 @@ #include "output_api.h" #include "utils.h" #include "timer.h" +#include "fd_util.h" #include @@ -152,7 +153,7 @@ fifo_open(struct fifo_data *fd, GError **error) if (!fifo_check(fd, error)) return false; - fd->input = open(fd->path, O_RDONLY|O_NONBLOCK); + fd->input = open_cloexec(fd->path, O_RDONLY|O_NONBLOCK); if (fd->input < 0) { g_set_error(error, fifo_output_quark(), errno, "Could not open FIFO \"%s\" for reading: %s", @@ -161,7 +162,7 @@ fifo_open(struct fifo_data *fd, GError **error) return false; } - fd->output = open(fd->path, O_WRONLY|O_NONBLOCK); + fd->output = open_cloexec(fd->path, O_WRONLY|O_NONBLOCK); if (fd->output < 0) { g_set_error(error, fifo_output_quark(), errno, "Could not open FIFO \"%s\" for writing: %s", diff --git a/src/output/httpd_output_plugin.c b/src/output/httpd_output_plugin.c index 5b0331257..6d5e99c57 100644 --- a/src/output/httpd_output_plugin.c +++ b/src/output/httpd_output_plugin.c @@ -25,6 +25,7 @@ #include "socket_util.h" #include "page.h" #include "icy_server.h" +#include "fd_util.h" #include @@ -187,14 +188,14 @@ httpd_listen_in_event(G_GNUC_UNUSED GIOChannel *source, struct httpd_output *httpd = data; int fd; struct sockaddr_storage sa; - socklen_t sa_length = sizeof(sa); + size_t sa_length = sizeof(sa); g_mutex_lock(httpd->mutex); /* the listener socket has become readable - a client has connected */ - fd = accept(httpd->fd, (struct sockaddr*)&sa, &sa_length); + fd = accept_cloexec(httpd->fd, (struct sockaddr*)&sa, &sa_length); if (fd >= 0) { /* can we allow additional client */ if (httpd->open && diff --git a/src/output/mvp_plugin.c b/src/output/mvp_plugin.c index 96f9435a8..f5fbadbee 100644 --- a/src/output/mvp_plugin.c +++ b/src/output/mvp_plugin.c @@ -22,7 +22,8 @@ * http://mvpmc.sourceforge.net/ */ -#include "../output_api.h" +#include "output_api.h" +#include "fd_util.h" #include @@ -115,7 +116,7 @@ mvp_output_test_default_device(void) { int fd; - fd = open("/dev/adec_pcm", O_WRONLY); + fd = open_cloexec("/dev/adec_pcm", O_WRONLY); if (fd >= 0) { close(fd); @@ -230,7 +231,8 @@ mvp_output_open(void *data, struct audio_format *audio_format, GError **error) int mix[5] = { 0, 2, 7, 1, 0 }; bool success; - if ((md->fd = open("/dev/adec_pcm", O_RDWR | O_NONBLOCK)) < 0) { + md->fd = open_cloexec("/dev/adec_pcm", O_RDWR | O_NONBLOCK); + if (md->fd < 0) { g_set_error(error, mvp_output_quark(), errno, "Error opening /dev/adec_pcm: %s", strerror(errno)); diff --git a/src/output/oss_plugin.c b/src/output/oss_plugin.c index 258fbfa9d..f308c293e 100644 --- a/src/output/oss_plugin.c +++ b/src/output/oss_plugin.c @@ -17,8 +17,9 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "../output_api.h" +#include "output_api.h" #include "mixer_list.h" +#include "fd_util.h" #include @@ -343,7 +344,9 @@ oss_output_test_default_device(void) int fd, i; for (i = G_N_ELEMENTS(default_devices); --i >= 0; ) { - if ((fd = open(default_devices[i], O_WRONLY)) >= 0) { + fd = open_cloexec(default_devices[i], O_WRONLY); + + if (fd >= 0) { close(fd); return true; } @@ -516,7 +519,8 @@ oss_open(struct oss_data *od, GError **error) { bool success; - if ((od->fd = open(od->device, O_WRONLY)) < 0) { + od->fd = open_cloexec(od->device, O_WRONLY); + if (od->fd < 0) { g_set_error(error, oss_output_quark(), errno, "Error opening OSS device \"%s\": %s", od->device, strerror(errno)); diff --git a/src/output/recorder_output_plugin.c b/src/output/recorder_output_plugin.c index 413e5d0d1..202b56073 100644 --- a/src/output/recorder_output_plugin.c +++ b/src/output/recorder_output_plugin.c @@ -20,6 +20,7 @@ #include "output_api.h" #include "encoder_plugin.h" #include "encoder_list.h" +#include "fd_util.h" #include #include @@ -156,7 +157,7 @@ recorder_output_open(void *data, struct audio_format *audio_format, /* create the output file */ - recorder->fd = creat(recorder->path, 0666); + recorder->fd = creat_cloexec(recorder->path, 0666); if (recorder->fd < 0) { g_set_error(error_r, recorder_output_quark(), 0, "Failed to create '%s': %s", diff --git a/src/output/solaris_output_plugin.c b/src/output/solaris_output_plugin.c index 5febf0afc..4f3d86835 100644 --- a/src/output/solaris_output_plugin.c +++ b/src/output/solaris_output_plugin.c @@ -18,6 +18,7 @@ */ #include "output_api.h" +#include "fd_util.h" #include @@ -91,7 +92,7 @@ solaris_output_open(void *data, struct audio_format *audio_format, /* open the device in non-blocking mode */ - so->fd = open(so->device, O_WRONLY|O_NONBLOCK); + so->fd = open_cloexec(so->device, O_WRONLY|O_NONBLOCK); if (so->fd < 0) { g_set_error(error, solaris_output_quark(), errno, "Failed to open %s: %s", diff --git a/src/socket_util.c b/src/socket_util.c index da4e414b6..edbc67d12 100644 --- a/src/socket_util.c +++ b/src/socket_util.c @@ -18,6 +18,7 @@ */ #include "socket_util.h" +#include "fd_util.h" #include "config.h" #include @@ -102,7 +103,7 @@ socket_bind_listen(int domain, int type, int protocol, int passcred = 1; #endif - fd = socket(domain, type, protocol); + fd = socket_cloexec(domain, type, protocol); if (fd < 0) { g_set_error(error, listen_quark(), errno, "Failed to create socket: %s", g_strerror(errno)); -- cgit v1.2.3