Blob Blame History Raw
autofs-5.1.3 - move open_xxxx() functions to spawn.c

From: Ian Kent <raven@themaw.net>

In a bug report by John Salmon has described a race between the
autofs open_xxxx() functions and fork(2) that can cause file handles
that don't have close on exec set to leak into subprocesses.

Basically, in some cases there can be a finite time between performing
the open and setting the descriptor close on exec and it's this window
that leads to the problem.

The description went on to talk about a case where autofs can hang when
this happens. That is when the leaked decriptor causes poll(2) to not
return in another process because it is waiting for the decriptor to
close (on mount completion) but another execed process has the cloned
descriptor open.

Serializing access to the open_xxxx() functions wrt. to fork(2) calls
should be suficient to resolve this leakage.

This patch starts this by moving the open_xxxx() out of the automount.h
include file and into spawn.c.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 CHANGELOG           |    1 
 daemon/automount.c  |    3 -
 daemon/spawn.c      |  125 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/automount.h |  132 ++--------------------------------------------------
 4 files changed, 133 insertions(+), 128 deletions(-)

--- autofs-5.0.7.orig/CHANGELOG
+++ autofs-5.0.7/CHANGELOG
@@ -287,6 +287,7 @@
 - fix incorrect check in validate_program_options().
 - update configure to check for pipe2(2).
 - fix open calls not using open_xxxx() calls.
+- move open_xxxx() functions to spawn.c.
 
 25/07/2012 autofs-5.0.7
 =======================
--- autofs-5.0.7.orig/daemon/spawn.c
+++ autofs-5.0.7/daemon/spawn.c
@@ -49,6 +49,131 @@ void dump_core(void)
 }
 
 /*
+ * Use CLOEXEC flag for open(), pipe(), fopen() (read-only case) and
+ * socket() if possible.
+ */
+static int cloexec_works = 0;
+
+static void check_cloexec(int fd)
+{
+	if (cloexec_works == 0) {
+		int fl = fcntl(fd, F_GETFD);
+		if (fl != -1)
+			cloexec_works = (fl & FD_CLOEXEC) ? 1 : -1;
+	}
+	if (cloexec_works > 0)
+		return;
+	fcntl(fd, F_SETFD, FD_CLOEXEC);
+	return;
+}
+
+int open_fd(const char *path, int flags)
+{
+	int fd;
+
+#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
+	if (cloexec_works != -1)
+		flags |= O_CLOEXEC;
+#endif
+	fd = open(path, flags);
+	if (fd == -1)
+		return -1;
+	check_cloexec(fd);
+	return fd;
+}
+
+int open_fd_mode(const char *path, int flags, int mode)
+{
+	int fd;
+
+#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
+	if (cloexec_works != -1)
+		flags |= O_CLOEXEC;
+#endif
+	fd = open(path, flags, mode);
+	if (fd == -1)
+		return -1;
+	check_cloexec(fd);
+	return fd;
+}
+
+int open_pipe(int pipefd[2])
+{
+	int ret;
+
+#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC) && defined(HAVE_PIPE2)
+	if (cloexec_works != -1) {
+		ret = pipe2(pipefd, O_CLOEXEC);
+		if (ret != -1)
+			return 0;
+		if (errno != EINVAL)
+			return -1;
+	}
+#endif
+	ret = pipe(pipefd);
+	if (ret == -1)
+		return -1;
+	check_cloexec(pipefd[0]);
+	check_cloexec(pipefd[1]);
+	return 0;
+}
+
+int open_sock(int domain, int type, int protocol)
+{
+	int fd;
+
+#ifdef SOCK_CLOEXEC
+	if (cloexec_works != -1)
+		type |= SOCK_CLOEXEC;
+#endif
+	fd = socket(domain, type, protocol);
+	if (fd == -1)
+		return -1;
+	check_cloexec(fd);
+	return fd;
+}
+
+FILE *open_fopen_r(const char *path)
+{
+	FILE *f;
+
+#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
+	if (cloexec_works != -1) {
+		f = fopen(path, "re");
+		if (f != NULL) {
+			check_cloexec(fileno(f));
+			return f;
+		}
+	}
+#endif
+	f = fopen(path, "r");
+	if (f == NULL)
+		return NULL;
+	check_cloexec(fileno(f));
+	return f;
+}
+
+FILE *open_setmntent_r(const char *table)
+{
+	FILE *tab;
+
+#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
+	if (cloexec_works != -1) {
+		tab = setmntent(table, "re");
+		if (tab != NULL) {
+			check_cloexec(fileno(tab));
+			return tab;
+		}
+	}
+#endif
+	tab = fopen(table, "r");
+	if (tab == NULL)
+		return NULL;
+	check_cloexec(fileno(tab));
+	return tab;
+}
+
+/*
  * Used by subprocesses which exec to avoid carrying over the main
  * daemon's signalling environment
  */
--- autofs-5.0.7.orig/include/automount.h
+++ autofs-5.0.7/include/automount.h
@@ -8,6 +8,7 @@
 #ifndef AUTOMOUNT_H
 #define AUTOMOUNT_H
 
+#include <stdio.h>
 #include <paths.h>
 #include <limits.h>
 #include <time.h>
@@ -256,6 +257,12 @@ int spawnv(unsigned logopt, const char *
 int spawn_mount(unsigned logopt, ...);
 int spawn_bind_mount(unsigned logopt, ...);
 int spawn_umount(unsigned logopt, ...);
+int open_fd(const char *, int);
+int open_fd_mode(const char *, int, int);
+int open_pipe(int[2]);
+int open_sock(int, int, int);
+FILE *open_fopen_r(const char *);
+FILE *open_setmntent_r(const char *);
 void reset_signals(void);
 int do_mount(struct autofs_point *ap, const char *root, const char *name,
 	     int name_len, const char *what, const char *fstype,
@@ -600,130 +607,5 @@ int alarm_start_handler(void);
 int alarm_add(struct autofs_point *ap, time_t seconds);
 void alarm_delete(struct autofs_point *ap);
 
-/*
- * Use CLOEXEC flag for open(), pipe(), fopen() (read-only case) and
- * socket() if possible.
- */
-static int cloexec_works;
-
-static inline void check_cloexec(int fd)
-{
-	if (cloexec_works == 0) {
-		int fl = fcntl(fd, F_GETFD);
-		if (fl != -1)
-			cloexec_works = (fl & FD_CLOEXEC) ? 1 : -1;
-	}
-	if (cloexec_works > 0)
-		return;
-	fcntl(fd, F_SETFD, FD_CLOEXEC);
-	return;
-}
-
-static inline int open_fd(const char *path, int flags)
-{
-	int fd;
-
-#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
-	if (cloexec_works != -1)
-		flags |= O_CLOEXEC;
-#endif
-	fd = open(path, flags);
-	if (fd == -1)
-		return -1;
-	check_cloexec(fd);
-	return fd;
-}
-
-static inline int open_fd_mode(const char *path, int flags, int mode)
-{
-	int fd;
-
-#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
-	if (cloexec_works != -1)
-		flags |= O_CLOEXEC;
-#endif
-	fd = open(path, flags, mode);
-	if (fd == -1)
-		return -1;
-	check_cloexec(fd);
-	return fd;
-}
-
-static inline int open_pipe(int pipefd[2])
-{
-	int ret;
-
-#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC) && defined(HAVE_PIPE2)
-	if (cloexec_works != -1) {
-		ret = pipe2(pipefd, O_CLOEXEC);
-		if (ret != -1)
-			return 0;
-		if (errno != EINVAL)
-			return -1;
-	}
-#endif
-	ret = pipe(pipefd);
-	if (ret == -1)
-		return -1;
-	check_cloexec(pipefd[0]);
-	check_cloexec(pipefd[1]);
-	return 0;
-}
-
-static inline int open_sock(int domain, int type, int protocol)
-{
-	int fd;
-
-#ifdef SOCK_CLOEXEC
-	if (cloexec_works != -1)
-		type |= SOCK_CLOEXEC;
-#endif
-	fd = socket(domain, type, protocol);
-	if (fd == -1)
-		return -1;
-	check_cloexec(fd);
-	return fd;
-}
-
-static inline FILE *open_fopen_r(const char *path)
-{
-	FILE *f;
-
-#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
-	if (cloexec_works != -1) {
-		f = fopen(path, "re");
-		if (f != NULL) {
-			check_cloexec(fileno(f));
-			return f;
-		}
-	}
-#endif
-	f = fopen(path, "r");
-	if (f == NULL)
-		return NULL;
-	check_cloexec(fileno(f));
-	return f;
-}
-
-static inline FILE *open_setmntent_r(const char *table)
-{
-	FILE *tab;
-
-#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
-	if (cloexec_works != -1) {
-		tab = setmntent(table, "re");
-		if (tab != NULL) {
-			check_cloexec(fileno(tab));
-			return tab;
-		}
-	}
-#endif
-	tab = fopen(table, "r");
-	if (tab == NULL)
-		return NULL;
-	check_cloexec(fileno(tab));
-	return tab;
-}
-
 #endif
 
--- autofs-5.0.7.orig/daemon/automount.c
+++ autofs-5.0.7/daemon/automount.c
@@ -75,9 +75,6 @@ static sigset_t block_sigs;
 /* Pre-calculated kernel packet length */
 static size_t kpkt_len;
 
-/* Does kernel know about SOCK_CLOEXEC and friends */
-static int cloexec_works = 0;
-
 /* Attributes for creating detached and joinable threads */
 pthread_attr_t th_attr;
 pthread_attr_t th_attr_detached;