search

canonical / mir

The Mir compositor
GNU General Public License v2.0
620 stars 99 forks source link

Our `fork`/`exec` spawning is unsafe #3494

Open RAOF opened 1 month ago

RAOF commented 1 month ago

exec_wayland() is called after fork(): https://github.com/canonical/mir/blob/953de04e07444a8f4410dd2f044ad9d24fbec2ef/src/server/frontend_xwayland/xwayland_server.cpp#L53-L98

Unfortunately, man 2 fork says:

After a fork() in a multithreaded program, the child can safely call only async-signal-safe functions (see signal-safety(7)) until such time as it calls execve(2).

We should instead do as much setup as possible in the parent, and then call fork(). (Amusingly, even execvp is not async-signal-safe :woman_facepalming: )

AlanGriffiths commented 1 month ago

Amusingly, even execvp is not async-signal-safe

That probably doesn't matter: we should use execve() directly (we default xwayland-path to "/usr/bin/Xwayland", which doesn't need PATH handling)

RAOF commented 1 month ago

Oops! When reviewing https://github.com/canonical/mir/pull/3497 I also see that miral::launch_app_env suffers from the same problems.