[PATCH weston] launcher-util: Force all weston_launcher_open()s to use O_CLOEXEC

Derek Foreman derekf at osg.samsung.com
Fri May 1 09:46:36 PDT 2015


Really, there's pretty much no time we'd ever want O_CLOEXEC unset,
as it will likely result in leaking fds to processes that aren't
interested in them or shouldn't have them.

This also removes the (now unused) code from weston_logind_open() that
could drop O_CLOEXEC.

Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
---
Daniel suggested I do something like this instead of fixing all the
callers as I did in my previous patches.

I think this should supersede my previous two patches on the subject.

It's still possible for someone to leak fds to child processes if they
really want to (by using fcntl to clear O_CLOEXEC).  I really can't think
of a use case for that.  By making it much harder to do on purpose it's
become almost impossible to do by accident...

 src/launcher-util.c |  8 +++++++-
 src/logind-util.c   | 20 ++------------------
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/src/launcher-util.c b/src/launcher-util.c
index e89710b..6ff3783 100644
--- a/src/launcher-util.c
+++ b/src/launcher-util.c
@@ -114,11 +114,17 @@ weston_launcher_open(struct weston_launcher *launcher,
 	struct weston_launcher_open *message;
 	struct stat s;
 
+	/* We really don't want to be leaking fds to child processes so
+	 * we force this flag here.  If someone comes up with a legitimate
+	 * reason to not CLOEXEC they'll need to unset the flag manually.
+	 */
+	flags |= O_CLOEXEC;
+
 	if (launcher->logind)
 		return weston_logind_open(launcher->logind, path, flags);
 
 	if (launcher->fd == -1) {
-		fd = open(path, flags | O_CLOEXEC);
+		fd = open(path, flags);
 		if (fd == -1)
 			return -1;
 
diff --git a/src/logind-util.c b/src/logind-util.c
index e4e20eb..f6514f1 100644
--- a/src/logind-util.c
+++ b/src/logind-util.c
@@ -186,8 +186,8 @@ weston_logind_open(struct weston_logind *wl, const char *path,
 	 * directly. Instead, logind passes us an fd with sane default modes.
 	 * For DRM and evdev this means O_RDWR | O_CLOEXEC. If we want
 	 * something else, we need to change it afterwards. We currently
-	 * only support dropping FD_CLOEXEC and setting O_NONBLOCK. Changing
-	 * access-modes is not possible so accept whatever logind passes us. */
+	 * only support setting O_NONBLOCK. Changing access-modes is not
+	 * possible so accept whatever logind passes us. */
 
 	fl = fcntl(fd, F_GETFL);
 	if (fl < 0) {
@@ -203,22 +203,6 @@ weston_logind_open(struct weston_logind *wl, const char *path,
 		r = -errno;
 		goto err_close;
 	}
-
-	fl = fcntl(fd, F_GETFD);
-	if (fl < 0) {
-		r = -errno;
-		goto err_close;
-	}
-
-	if (!(flags & O_CLOEXEC))
-		fl &= ~FD_CLOEXEC;
-
-	r = fcntl(fd, F_SETFD, fl);
-	if (r < 0) {
-		r = -errno;
-		goto err_close;
-	}
-
 	return fd;
 
 err_close:
-- 
2.1.4



More information about the wayland-devel mailing list