config: Fix socket/wl_client leaks when executing child processes

Added spawn_wl_client helper function that is used for spawning both bar and
background processes

include/sway/util.h

@@ -0,0 +1,14 @@
+#ifndef _SWAY_SWAY_UTIL_H
+#define _SWAY_SWAY_UTIL_H
+
+#include <spawn.h>
+
+/**
+ * Close fd and log theoretical case when close(2) failed
+ */
+void close_warn(int fd);
+
+struct wl_client *spawn_wl_client(char * const cmd[], struct wl_display *display);
+struct wl_client *spawn_wl_client_fa(char * const cmd[], struct wl_display *display, posix_spawn_file_actions_t *fa);
+
+#endif//_SWAY_SWAY_UTIL_H

sway/config/bar.c

@@ -13,6 +13,7 @@
 #include "sway/config.h"
 #include "sway/input/keyboard.h"
 #include "sway/output.h"
+#include "sway/util.h"
 #include "config.h"
 #include "list.h"
 #include "log.h"
@@ -187,79 +188,25 @@ static void handle_swaybar_client_destroy(struct wl_listener *listener,
 	bar->client = NULL;
 }
 
-static void invoke_swaybar(struct bar_config *bar) {
-	int sockets[2];
-	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) != 0) {
-		sway_log_errno(SWAY_ERROR, "socketpair failed");
-		return;
-	}
-	if (!set_cloexec(sockets[0], true) || !set_cloexec(sockets[1], true)) {
-		return;
-	}
-
-	bar->client = wl_client_create(server.wl_display, sockets[0]);
-	if (bar->client == NULL) {
-		sway_log_errno(SWAY_ERROR, "wl_client_create failed");
-		return;
-	}
-
-	bar->client_destroy.notify = handle_swaybar_client_destroy;
-	wl_client_add_destroy_listener(bar->client, &bar->client_destroy);
-
-	pid_t pid = fork();
-	if (pid < 0) {
-		sway_log(SWAY_ERROR, "Failed to create fork for swaybar");
-		return;
-	} else if (pid == 0) {
-		// Remove the SIGUSR1 handler that wlroots adds for xwayland
-		sigset_t set;
-		sigemptyset(&set);
-		sigprocmask(SIG_SETMASK, &set, NULL);
-
-		pid = fork();
-		if (pid < 0) {
-			sway_log_errno(SWAY_ERROR, "fork failed");
-			_exit(EXIT_FAILURE);
-		} else if (pid == 0) {
-			if (!set_cloexec(sockets[1], false)) {
-				_exit(EXIT_FAILURE);
-			}
-
-			char wayland_socket_str[16];
-			snprintf(wayland_socket_str, sizeof(wayland_socket_str),
-					"%d", sockets[1]);
-			setenv("WAYLAND_SOCKET", wayland_socket_str, true);
-
-			// run custom swaybar
-			char *const cmd[] = {
-					bar->swaybar_command ? bar->swaybar_command : "swaybar",
-					"-b", bar->id, NULL};
-			execvp(cmd[0], cmd);
-			_exit(EXIT_FAILURE);
-		}
-		_exit(EXIT_SUCCESS);
-	}
-
-	if (close(sockets[1]) != 0) {
-		sway_log_errno(SWAY_ERROR, "close failed");
-		return;
-	}
-
-	if (waitpid(pid, NULL, 0) < 0) {
-		sway_log_errno(SWAY_ERROR, "waitpid failed");
-		return;
-	}
-
-	sway_log(SWAY_DEBUG, "Spawned swaybar %s", bar->id);
-	return;
-}
-
 void load_swaybar(struct bar_config *bar) {
 	if (bar->client != NULL) {
 		wl_client_destroy(bar->client);
 	}
 	sway_log(SWAY_DEBUG, "Invoking swaybar for bar id '%s'", bar->id);
-	invoke_swaybar(bar);
+
+	char *const cmd[] = {
+			bar->swaybar_command ? bar->swaybar_command : "swaybar",
+			"-b", bar->id, NULL};
+	bar->client = spawn_wl_client(cmd, server.wl_display);
+
+	if (bar->client == NULL) {
+		sway_log(SWAY_ERROR, "Failed to spawn swaybar %s", bar->id);
+		return;
+	}
+
+	sway_log(SWAY_DEBUG, "Spawned swaybar %s", bar->id);
+	bar->client_destroy.notify = handle_swaybar_client_destroy;
+	wl_client_add_destroy_listener(bar->client, &bar->client_destroy);
 }
 
 void load_swaybars(void) {

sway/config/output.c

@@ -10,6 +10,7 @@
 #include "sway/config.h"
 #include "sway/output.h"
 #include "sway/tree/root.h"
+#include "sway/util.h"
 #include "log.h"
 #include "util.h"
 
@@ -489,67 +490,6 @@ static void handle_swaybg_client_destroy(struct wl_listener *listener,
 	sway_config->swaybg_client = NULL;
 }
 
-static bool _spawn_swaybg(char **command) {
-	if (config->swaybg_client != NULL) {
-		wl_client_destroy(config->swaybg_client);
-	}
-	int sockets[2];
-	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) != 0) {
-		sway_log_errno(SWAY_ERROR, "socketpair failed");
-		return false;
-	}
-	if (!set_cloexec(sockets[0], true) || !set_cloexec(sockets[1], true)) {
-		return false;
-	}
-
-	config->swaybg_client = wl_client_create(server.wl_display, sockets[0]);
-	if (config->swaybg_client == NULL) {
-		sway_log_errno(SWAY_ERROR, "wl_client_create failed");
-		return false;
-	}
-
-	config->swaybg_client_destroy.notify = handle_swaybg_client_destroy;
-	wl_client_add_destroy_listener(config->swaybg_client,
-		&config->swaybg_client_destroy);
-
-	pid_t pid = fork();
-	if (pid < 0) {
-		sway_log_errno(SWAY_ERROR, "fork failed");
-		return false;
-	} else if (pid == 0) {
-		pid = fork();
-		if (pid < 0) {
-			sway_log_errno(SWAY_ERROR, "fork failed");
-			_exit(EXIT_FAILURE);
-		} else if (pid == 0) {
-			if (!set_cloexec(sockets[1], false)) {
-				_exit(EXIT_FAILURE);
-			}
-
-			char wayland_socket_str[16];
-			snprintf(wayland_socket_str, sizeof(wayland_socket_str),
-				"%d", sockets[1]);
-			setenv("WAYLAND_SOCKET", wayland_socket_str, true);
-
-			execvp(command[0], command);
-			sway_log_errno(SWAY_ERROR, "execvp failed");
-			_exit(EXIT_FAILURE);
-		}
-		_exit(EXIT_SUCCESS);
-	}
-
-	if (close(sockets[1]) != 0) {
-		sway_log_errno(SWAY_ERROR, "close failed");
-		return false;
-	}
-	if (waitpid(pid, NULL, 0) < 0) {
-		sway_log_errno(SWAY_ERROR, "waitpid failed");
-		return false;
-	}
-
-	return true;
-}
-
 bool spawn_swaybg(void) {
 	if (!config->swaybg_command) {
 		return true;
@@ -607,7 +547,17 @@ bool spawn_swaybg(void) {
 		sway_log(SWAY_DEBUG, "spawn_swaybg cmd[%zd] = %s", k, cmd[k]);
 	}
 
-	bool result = _spawn_swaybg(cmd);
+	if (config->swaybg_client) {
+		wl_client_destroy(config->swaybg_client);
+	}
+
+	config->swaybg_client = spawn_wl_client(cmd, server.wl_display);
+
+	if (config->swaybg_client) {
+		config->swaybg_client_destroy.notify = handle_swaybg_client_destroy;
+		wl_client_add_destroy_listener(config->swaybg_client, &config->swaybg_client_destroy);
+	}
+
 	free(cmd);
-	return result;
+	return config->swaybg_client != NULL;
 }

sway/meson.build

@@ -9,6 +9,7 @@ sway_sources = files(
 	'security.c',
 	'server.c',
 	'swaynag.c',
+	'util.c',
 	'xdg_decoration.c',
 
 	'desktop/desktop.c',

sway/swaynag.c

@@ -10,6 +10,7 @@
 #include "log.h"
 #include "sway/server.h"
 #include "sway/swaynag.h"
+#include "sway/util.h"
 #include "util.h"
 
 static void handle_swaynag_client_destroy(struct wl_listener *listener,
@@ -21,6 +22,40 @@ static void handle_swaynag_client_destroy(struct wl_listener *listener,
 	swaynag->client = NULL;
 }
 
+static struct wl_client *swaynag_spawn_detailed(char *cmd[], struct swaynag_instance *swaynag) {
+	if (pipe(swaynag->fd) != 0) {
+		sway_log(SWAY_ERROR, "Failed to create pipe for swaynag");
+		return NULL;
+	}
+
+	if (!set_cloexec(swaynag->fd[1], true)) {
+		goto failed;
+	}
+
+	posix_spawn_file_actions_t fa;
+	posix_spawn_file_actions_init(&fa);
+	posix_spawn_file_actions_adddup2(&fa, swaynag->fd[0], STDIN_FILENO);
+	posix_spawn_file_actions_addclose(&fa, swaynag->fd[0]);
+
+	struct wl_client *client = spawn_wl_client_fa(cmd, server.wl_display, &fa);
+
+	posix_spawn_file_actions_destroy(&fa);
+
+	if (client == NULL) {
+		goto failed;
+	}
+
+	close_warn(swaynag->fd[0]);
+
+	return client;
+
+failed:
+	close_warn(swaynag->fd[0]);
+	close_warn(swaynag->fd[1]);
+
+	return NULL;
+}
+
 bool swaynag_spawn(const char *swaynag_command,
 		struct swaynag_instance *swaynag) {
 	if (swaynag->client != NULL) {
@@ -31,99 +66,28 @@ bool swaynag_spawn(const char *swaynag_command,
 		return true;
 	}
 
+	size_t length = strlen(swaynag_command) + strlen(swaynag->args) + 2;
+	char *swaynag_cmd = malloc(length);
+	snprintf(swaynag_cmd, length, "%s %s", swaynag_command, swaynag->args);
+	char *cmd[] = {"/bin/sh", "-c", swaynag_cmd, NULL};
+
 	if (swaynag->detailed) {
-		if (pipe(swaynag->fd) != 0) {
+		swaynag->client = swaynag_spawn_detailed(cmd, swaynag);
-			sway_log(SWAY_ERROR, "Failed to create pipe for swaynag");
+	} else {
-			return false;
+		swaynag->client = spawn_wl_client(cmd, server.wl_display);
-		}
-		if (!set_cloexec(swaynag->fd[1], true)) {
-			goto failed;
-		}
 	}
 
-	int sockets[2];
+	free(swaynag_cmd);
-	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) != 0) {
-		sway_log_errno(SWAY_ERROR, "socketpair failed");
-		goto failed;
-	}
-	if (!set_cloexec(sockets[0], true) || !set_cloexec(sockets[1], true)) {
-		goto failed;
-	}
 
-	swaynag->client = wl_client_create(server.wl_display, sockets[0]);
 	if (swaynag->client == NULL) {
-		sway_log_errno(SWAY_ERROR, "wl_client_create failed");
+		sway_log(SWAY_ERROR, "Failed to spawn process for swaynag");
-		goto failed;
+		return false;
 	}
 
 	swaynag->client_destroy.notify = handle_swaynag_client_destroy;
 	wl_client_add_destroy_listener(swaynag->client, &swaynag->client_destroy);
 
-	pid_t pid = fork();
-	if (pid < 0) {
-		sway_log(SWAY_ERROR, "Failed to create fork for swaynag");
-		goto failed;
-	} else if (pid == 0) {
-		pid = fork();
-		if (pid < 0) {
-			sway_log_errno(SWAY_ERROR, "fork failed");
-			_exit(EXIT_FAILURE);
-		} else if (pid == 0) {
-			if (!set_cloexec(sockets[1], false)) {
-				_exit(EXIT_FAILURE);
-			}
-
-			if (swaynag->detailed) {
-				close(swaynag->fd[1]);
-				dup2(swaynag->fd[0], STDIN_FILENO);
-				close(swaynag->fd[0]);
-			}
-
-			char wayland_socket_str[16];
-			snprintf(wayland_socket_str, sizeof(wayland_socket_str),
-					"%d", sockets[1]);
-			setenv("WAYLAND_SOCKET", wayland_socket_str, true);
-
-			size_t length = strlen(swaynag_command) + strlen(swaynag->args) + 2;
-			char *cmd = malloc(length);
-			snprintf(cmd, length, "%s %s", swaynag_command, swaynag->args);
-			execl("/bin/sh", "/bin/sh", "-c", cmd, NULL);
-			sway_log_errno(SWAY_ERROR, "execl failed");
-			_exit(EXIT_FAILURE);
-		}
-		_exit(EXIT_SUCCESS);
-	}
-
-	if (swaynag->detailed) {
-		if (close(swaynag->fd[0]) != 0) {
-			sway_log_errno(SWAY_ERROR, "close failed");
-			return false;
-		}
-	}
-
-	if (close(sockets[1]) != 0) {
-		sway_log_errno(SWAY_ERROR, "close failed");
-		return false;
-	}
-
-	if (waitpid(pid, NULL, 0) < 0) {
-		sway_log_errno(SWAY_ERROR, "waitpid failed");
-		return false;
-	}
-
 	return true;
-
-failed:
-	if (swaynag->detailed) {
-		if (close(swaynag->fd[0]) != 0) {
-			sway_log_errno(SWAY_ERROR, "close failed");
-			return false;
-		}
-		if (close(swaynag->fd[1]) != 0) {
-			sway_log_errno(SWAY_ERROR, "close failed");
-		}
-	}
-	return false;
 }
 
 void swaynag_log(const char *swaynag_command, struct swaynag_instance *swaynag,

sway/util.c

@@ -0,0 +1,96 @@
+#define _POSIX_C_SOURCE 200809L
+#include <signal.h>
+#include <spawn.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <wayland-server.h>
+
+#include "log.h"
+#include "sway/util.h"
+#include "util.h"
+
+extern char **environ;
+
+void close_warn(int fd) {
+	if (close(fd) != 0) {
+		sway_log_errno(SWAY_ERROR, "close failed");
+	}
+}
+
+struct wl_client *spawn_wl_client(char * const cmd[], struct wl_display *display) {
+	return spawn_wl_client_fa(cmd, display, NULL);
+}
+
+struct wl_client *spawn_wl_client_fa(char * const cmd[], struct wl_display *display, posix_spawn_file_actions_t *fa) {
+	int sockets[2];
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) != 0) {
+		sway_log_errno(SWAY_ERROR, "socketpair failed");
+		return NULL;
+	}
+
+	if (!set_cloexec(sockets[0], true) || !set_cloexec(sockets[1], true)) {
+		goto cleanup_sockets;
+	}
+
+	struct wl_client *client = wl_client_create(display, sockets[0]);
+	if (client == NULL) {
+		sway_log_errno(SWAY_ERROR, "wl_client_create failed");
+		goto cleanup_sockets;
+	}
+
+	pid_t pid = fork();
+	if (pid < 0) {
+		sway_log(SWAY_ERROR, "Failed to create fork for swaybar");
+		goto cleanup_client;
+	} else if (pid == 0) {
+		posix_spawnattr_t attr;
+		posix_spawnattr_init(&attr);
+
+		// Remove the SIGUSR1 handler that wlroots adds for xwayland
+		sigset_t set;
+		sigfillset(&set);
+		posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF);
+		posix_spawnattr_setsigdefault(&attr, &set);
+
+		char wayland_socket_str[16];
+		snprintf(wayland_socket_str, sizeof(wayland_socket_str),
+				"%d", sockets[1]);
+		setenv("WAYLAND_SOCKET", wayland_socket_str, true);
+
+		if (!set_cloexec(sockets[1], false)) {
+			_exit(EXIT_FAILURE);
+		}
+
+		int r = posix_spawnp(&pid, cmd[0], fa, &attr, cmd, environ);
+		if (r) {
+			sway_log_errno(SWAY_ERROR, "posix_spawnp failed");
+			_exit(EXIT_FAILURE);
+		}
+
+		posix_spawnattr_destroy(&attr);
+
+		_exit(EXIT_SUCCESS);
+	}
+
+	if (waitpid(pid, NULL, 0) < 0) {
+		sway_log_errno(SWAY_ERROR, "waitpid failed");
+		goto cleanup_client;
+	}
+
+	close_warn(sockets[1]);
+
+	return client;
+
+cleanup_client:
+	wl_client_destroy(client);
+
+cleanup_sockets:
+	close(sockets[0]);
+	close(sockets[1]);
+
+	return NULL;
+}