From 53f895b4764451b6852766e0c597d9aff807fc35 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Tue, 12 Jul 2022 17:51:49 +0100 Subject: [PATCH] wet_process: Do not weston_log() after fork() [common equivalent of 77cf8cb0064c in Xwayland from Pekka Paalanen; its commit message follows] Between fork() and exec() in the child process it is only safe to use async-signal-safe functions. weston_log() definitely is not one, it allocates memory and does whatnot. weston_log() is also inappropriate for other reasons: the child process has its own stream buffers and flight-recorder. No-one looks into the child process' flight recorder, so messages would be lost there. The logging machinery might also attempt to write into debug streams, meaning both parent and child could be writing simultaneously. It seems that the best we can do is to pre-bake an error message and only write() it out if exec() fails. There is no mention that even strerror_r() might be safe to call, so we don't. Signed-off-by: Daniel Stone --- compositor/main.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/compositor/main.c b/compositor/main.c index da62684c..280b565e 100644 --- a/compositor/main.c +++ b/compositor/main.c @@ -377,6 +377,9 @@ weston_client_launch(struct weston_compositor *compositor, struct wl_client *client = NULL; struct custom_env child_env; struct fdstr wayland_socket; + const char *fail_cloexec = "Couldn't unset CLOEXEC on client socket"; + const char *fail_seteuid = "Couldn't call seteuid"; + char *fail_exec; char * const *argp; char * const *envp; sigset_t allsigs; @@ -384,6 +387,7 @@ weston_client_launch(struct weston_compositor *compositor, bool ret; weston_log("launching '%s'\n", path); + str_printf(&fail_exec, "Error: Couldn't launch client '%s'\n", path); custom_env_init_from_environ(&child_env); custom_env_add_arg(&child_env, path); @@ -420,22 +424,22 @@ weston_client_launch(struct weston_compositor *compositor, /* Launch clients as the user. Do not launch clients with wrong euid. */ if (seteuid(getuid()) == -1) { - weston_log("compositor: failed seteuid\n"); + write(STDERR_FILENO, fail_seteuid, + strlen(fail_seteuid)); _exit(EXIT_FAILURE); } ret = fdstr_clear_cloexec_fd1(&wayland_socket); if (!ret) { - weston_log("compositor: clearing CLOEXEC failed: %s\n", - strerror(errno)); + write(STDERR_FILENO, fail_cloexec, + strlen(fail_cloexec)); _exit(EXIT_FAILURE); } - if (execve(argp[0], argp, envp)) { - weston_log("compositor: executing '%s' failed: %s\n", - path, strerror(errno)); - } + execve(argp[0], argp, envp); + if (fail_exec) + write(STDERR_FILENO, fail_exec, strlen(fail_exec)); _exit(EXIT_FAILURE); default: @@ -445,6 +449,7 @@ weston_client_launch(struct weston_compositor *compositor, if (!client) { custom_env_fini(&child_env); close(wayland_socket.fds[0]); + free(fail_exec); weston_log("weston_client_launch: " "wl_client_create failed while launching '%s'.\n", path); @@ -465,6 +470,7 @@ weston_client_launch(struct weston_compositor *compositor, } custom_env_fini(&child_env); + free(fail_exec); return client; }