[alsa-devel] [RESEND] [PATCH 0/3] Close alsa-lib file descriptors on exec
Hello,
The device nodes opened by alsa-lib are private, and are not meant to be used after exec(). Setting the close-on-exec flags avoid leaking devices file descriptors into unintended processes. Further more, the Linux kernel O_CLOEXEC flag is used if available for fully thread-safe operations.
configure.in | 2 ++ include/local.h | 23 +++++++++++++++++------ src/control/control_hw.c | 11 ----------- src/hwdep/hwdep_hw.c | 11 ----------- src/pcm/pcm_hw.c | 12 ------------ src/rawmidi/rawmidi_hw.c | 11 ----------- src/seq/seq_hw.c | 11 ----------- src/timer/timer_hw.c | 11 ----------- 8 files changed, 19 insertions(+), 73 deletions(-)
Signed-off-by: Rémi Denis-Courmont remi@remlab.net --- include/local.h | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/include/local.h b/include/local.h index b5a1c45..5b54def 100644 --- a/include/local.h +++ b/include/local.h @@ -230,22 +230,33 @@ extern snd_lib_error_handler_t snd_err_msg; # define link_warning(symbol, msg) #endif
-/* open with resmgr */ -#ifdef SUPPORT_RESMGR static inline int snd_open_device(const char *filename, int fmode) { - int fd = open(filename, fmode); + int fd; + +#ifdef O_CLOEXEC + fd = open(filename, fmode|O_CLOEXEC); if (fd >= 0) return fd; + if (errno == EINVAL) +#endif + { + fd = open(filename, fmode); + if (fd >= 0) { + fcntl(fd, F_SETFD, FD_CLOEXEC); + return fd; + } + } + +/* open with resmgr */ +#ifdef SUPPORT_RESMGR if (errno == EAGAIN || errno == EBUSY) return fd; if (! access(filename, F_OK)) return rsm_open_device(filename, fmode); +#endif return -1; } -#else -#define snd_open_device(filename, fmode) open(filename, fmode); -#endif
/* make local functions really local */ #define snd_dlobj_cache_lookup \
On Wed, 04.11.09 20:38, Rémi Denis-Courmont (remi@remlab.net) wrote:
static inline int snd_open_device(const char *filename, int fmode) {
- int fd = open(filename, fmode);
- int fd;
+#ifdef O_CLOEXEC
- fd = open(filename, fmode|O_CLOEXEC); if (fd >= 0) return fd;
- if (errno == EINVAL)
+#endif
- {
fd = open(filename, fmode);
if (fd >= 0) {
fcntl(fd, F_SETFD, FD_CLOEXEC);
return fd;
}
That's actually not how things works. O_CLOEXEC is silently ignored by old kernels. (The calls with SOCK_CLOEXEC fail with EINVAL while the calls with O_CLOEXEC silently succeed) You need to set FD_CLOEXEC unconditionally to cover all cases:
fd = open(fn, fmode #ifdef O_CLOEXEC |O_CLOEXEC #endif ); fcntl(fd, F_SETFD, FD_CLOEXEC);
Lennart
Signed-off-by: Rémi Denis-Courmont remi@remlab.net --- src/control/control_hw.c | 11 ----------- src/hwdep/hwdep_hw.c | 11 ----------- src/pcm/pcm_hw.c | 12 ------------ src/rawmidi/rawmidi_hw.c | 11 ----------- src/seq/seq_hw.c | 11 ----------- src/timer/timer_hw.c | 11 ----------- 6 files changed, 0 insertions(+), 67 deletions(-)
diff --git a/src/control/control_hw.c b/src/control/control_hw.c index e9a6be2..cf258b4 100644 --- a/src/control/control_hw.c +++ b/src/control/control_hw.c @@ -392,17 +392,6 @@ int snd_ctl_hw_open(snd_ctl_t **handle, const char *name, int card, int mode) if (fd < 0) return -errno; } -#if 0 - /* - * this is bogus, an application have to care about open filedescriptors - */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - SYSERR("fcntl FD_CLOEXEC failed"); - err = -errno; - close(fd); - return err; - } -#endif if (ioctl(fd, SNDRV_CTL_IOCTL_PVERSION, &ver) < 0) { err = -errno; close(fd); diff --git a/src/hwdep/hwdep_hw.c b/src/hwdep/hwdep_hw.c index 238a507..e4fcdc3 100644 --- a/src/hwdep/hwdep_hw.c +++ b/src/hwdep/hwdep_hw.c @@ -122,17 +122,6 @@ int snd_hwdep_hw_open(snd_hwdep_t **handle, const char *name, int card, int devi if (fd < 0) return -errno; } -#if 0 - /* - * this is bogus, an application have to care about open filedescriptors - */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - SYSERR("fcntl FD_CLOEXEC failed"); - ret = -errno; - close(fd); - return ret; - } -#endif if (ioctl(fd, SNDRV_HWDEP_IOCTL_PVERSION, &ver) < 0) { ret = -errno; close(fd); diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 8abb204..2095b01 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -1144,18 +1144,6 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, if (fmode & O_ASYNC) mode |= SND_PCM_ASYNC;
-#if 0 - /* - * this is bogus, an application have to care about open filedescriptors - */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - ret = -errno; - SYSMSG("fcntl FD_CLOEXEC failed"); - close(fd); - return ret; - } -#endif - if (ioctl(fd, SNDRV_PCM_IOCTL_PVERSION, &ver) < 0) { ret = -errno; SYSMSG("SNDRV_PCM_IOCTL_PVERSION failed"); diff --git a/src/rawmidi/rawmidi_hw.c b/src/rawmidi/rawmidi_hw.c index 87829fd..e08dca7 100644 --- a/src/rawmidi/rawmidi_hw.c +++ b/src/rawmidi/rawmidi_hw.c @@ -234,17 +234,6 @@ int snd_rawmidi_hw_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp, return -errno; } } -#if 0 - /* - * this is bogus, an application have to care about open filedescriptors - */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - SYSERR("fcntl FD_CLOEXEC failed"); - ret = -errno; - close(fd); - return ret; - } -#endif if (ioctl(fd, SNDRV_RAWMIDI_IOCTL_PVERSION, &ver) < 0) { ret = -errno; SYSERR("SNDRV_RAWMIDI_IOCTL_PVERSION failed"); diff --git a/src/seq/seq_hw.c b/src/seq/seq_hw.c index 0e94593..5bb26f3 100644 --- a/src/seq/seq_hw.c +++ b/src/seq/seq_hw.c @@ -457,17 +457,6 @@ int snd_seq_hw_open(snd_seq_t **handle, const char *name, int streams, int mode) SYSERR("open %s failed", filename); return -errno; } -#if 0 - /* - * this is bogus, an application have to care about open filedescriptors - */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - SYSERR("fcntl FD_CLOEXEC failed"); - ret = -errno; - close(fd); - return ret; - } -#endif if (ioctl(fd, SNDRV_SEQ_IOCTL_PVERSION, &ver) < 0) { SYSERR("SNDRV_SEQ_IOCTL_PVERSION failed"); ret = -errno; diff --git a/src/timer/timer_hw.c b/src/timer/timer_hw.c index 3453858..25f3bab 100644 --- a/src/timer/timer_hw.c +++ b/src/timer/timer_hw.c @@ -236,17 +236,6 @@ int snd_timer_hw_open(snd_timer_t **handle, const char *name, int dev_class, int fd = snd_open_device(SNDRV_FILE_TIMER, tmode); if (fd < 0) return -errno; -#if 0 - /* - * this is bogus, an application have to care about open filedescriptors - */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - SYSERR("fcntl FD_CLOEXEC failed"); - ret = -errno; - close(fd); - return ret; - } -#endif if (ioctl(fd, SNDRV_TIMER_IOCTL_PVERSION, &ver) < 0) { ret = -errno; close(fd);
Signed-off-by: Rémi Denis-Courmont remi@remlab.net --- configure.in | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/configure.in b/configure.in index a455de1..cc8950f 100644 --- a/configure.in +++ b/configure.in @@ -38,6 +38,8 @@ then AC_MSG_RESULT($CC) fi +CFLAGS="$CFLAGS -D_GNU_SOURCE" +
AC_PROG_CC AC_PROG_CPP
participants (3)
-
Lennart Poettering
-
Rémi Denis-Courmont
-
Rémi Denis-Courmont