wet_process: Do not weston_log() after fork()

[common equivalent of 77cf8cb006 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 <daniels@collabora.com>
dev
Daniel Stone 2 years ago committed by Pekka Paalanen
parent 9ab97ebd72
commit 53f895b476
  1. 20
      compositor/main.c

@ -377,6 +377,9 @@ weston_client_launch(struct weston_compositor *compositor,
struct wl_client *client = NULL; struct wl_client *client = NULL;
struct custom_env child_env; struct custom_env child_env;
struct fdstr wayland_socket; 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 *argp;
char * const *envp; char * const *envp;
sigset_t allsigs; sigset_t allsigs;
@ -384,6 +387,7 @@ weston_client_launch(struct weston_compositor *compositor,
bool ret; bool ret;
weston_log("launching '%s'\n", path); 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_init_from_environ(&child_env);
custom_env_add_arg(&child_env, path); 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. */ /* Launch clients as the user. Do not launch clients with wrong euid. */
if (seteuid(getuid()) == -1) { if (seteuid(getuid()) == -1) {
weston_log("compositor: failed seteuid\n"); write(STDERR_FILENO, fail_seteuid,
strlen(fail_seteuid));
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
} }
ret = fdstr_clear_cloexec_fd1(&wayland_socket); ret = fdstr_clear_cloexec_fd1(&wayland_socket);
if (!ret) { if (!ret) {
weston_log("compositor: clearing CLOEXEC failed: %s\n", write(STDERR_FILENO, fail_cloexec,
strerror(errno)); strlen(fail_cloexec));
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
} }
if (execve(argp[0], argp, envp)) { execve(argp[0], argp, envp);
weston_log("compositor: executing '%s' failed: %s\n",
path, strerror(errno));
}
if (fail_exec)
write(STDERR_FILENO, fail_exec, strlen(fail_exec));
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
default: default:
@ -445,6 +449,7 @@ weston_client_launch(struct weston_compositor *compositor,
if (!client) { if (!client) {
custom_env_fini(&child_env); custom_env_fini(&child_env);
close(wayland_socket.fds[0]); close(wayland_socket.fds[0]);
free(fail_exec);
weston_log("weston_client_launch: " weston_log("weston_client_launch: "
"wl_client_create failed while launching '%s'.\n", "wl_client_create failed while launching '%s'.\n",
path); path);
@ -465,6 +470,7 @@ weston_client_launch(struct weston_compositor *compositor,
} }
custom_env_fini(&child_env); custom_env_fini(&child_env);
free(fail_exec);
return client; return client;
} }

Loading…
Cancel
Save