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.
This commit is contained in:
Max Kellermann 2009-11-07 18:55:16 +01:00
parent 9b21152600
commit e3af0032b2
17 changed files with 271 additions and 24 deletions

View File

@ -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

1
NEWS
View File

@ -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

View File

@ -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) ]),,

View File

@ -19,6 +19,7 @@
#include "event_pipe.h"
#include "utils.h"
#include "fd_util.h"
#include <stdbool.h>
#include <assert.h>
@ -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

171
src/fd_util.c Normal file
View File

@ -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 <assert.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#ifdef WIN32
#include <winsock2.h>
#else
#include <sys/socket.h>
#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;
}

53
src/fd_util.h Normal file
View File

@ -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 <stdbool.h>
#include <stddef.h>
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

View File

@ -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 <sys/stat.h>
#include <fcntl.h>
@ -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",

View File

@ -22,6 +22,7 @@
#include "client.h"
#include "conf.h"
#include "utils.h"
#include "fd_util.h"
#include "config.h"
#include <sys/types.h>
@ -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);

View File

@ -20,6 +20,7 @@
#include "log.h"
#include "conf.h"
#include "utils.h"
#include "fd_util.h"
#include "config.h"
#include <assert.h>
@ -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

View File

@ -19,6 +19,7 @@
#include "mixer_api.h"
#include "output_api.h"
#include "fd_util.h"
#include <glib.h>
@ -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",

View File

@ -20,6 +20,7 @@
#include "output_api.h"
#include "utils.h"
#include "timer.h"
#include "fd_util.h"
#include <glib.h>
@ -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",

View File

@ -25,6 +25,7 @@
#include "socket_util.h"
#include "page.h"
#include "icy_server.h"
#include "fd_util.h"
#include <assert.h>
@ -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 &&

View File

@ -22,7 +22,8 @@
* http://mvpmc.sourceforge.net/
*/
#include "../output_api.h"
#include "output_api.h"
#include "fd_util.h"
#include <glib.h>
@ -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));

View File

@ -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 <glib.h>
@ -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));

View File

@ -20,6 +20,7 @@
#include "output_api.h"
#include "encoder_plugin.h"
#include "encoder_list.h"
#include "fd_util.h"
#include <assert.h>
#include <sys/types.h>
@ -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",

View File

@ -18,6 +18,7 @@
*/
#include "output_api.h"
#include "fd_util.h"
#include <glib.h>
@ -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",

View File

@ -18,6 +18,7 @@
*/
#include "socket_util.h"
#include "fd_util.h"
#include "config.h"
#include <errno.h>
@ -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));