github
/
mpv
mirror of https://github.com/mpv-player/mpv
1
Fork 0
Code Releases Activity

subprocess: replace posix_spawnp() with fork() 

This code runs posix_spawnp() within a fork() in some cases, in order to
"disown" processes which are meant as being started detached. But
posix_spawnp() is not marked as async-signal-safe, so what we do is not
allowed. It could for example cause deadlocks, depending on
implementation and luck at runtime. Turns out posix_spawnp() is useless
crap.

Replace it with "classic" fork() to ensure correctness.

We could probably use another mechanism to start a process "disowned"
than doing a double-fork(). The only problem with "disowning" a process
is calling setsid() (which posix_spawnp() didn't support, but maybe will
in newer revisions), and removing as as parent from the child process
(the double-fork() will make PID 1 the parent). But there is no good way
to either remove us as parent, or to "reap" the PID in a way that is
safe and less of a mess than the current code. This is because
POSIX/UNIX is a miserable heap of shit. (Less shit than "alternatives"
like win32, no doubt.)

Because POSIX/UNIX is a miserable heap of shit, execvp() is also not
specified as async-signal-safe. It's funny how you can run a full
fledged HTTP server in an async-signal-safe context, but not start a
shitty damn process. Unix is really, really, really extremely bad at
this process management stuff. So we reimplement execvp() in an
async-signal-safe way.

The new code assumes that CLOEXEC is a thing. Since POSIX/UNIX is such a
heap of shit, O_CLOEXEC and FD_CLOEXEC were (probably) added at
different times, but both must be present. io.h defines them to 0 if
they don't exist, and in this case the code will error out at runtime.
Surely we could do without CLOEXEC via fallback, but I'll do that only
if at least 1 bug is reported wrt. this issue.

The idea how to report exec() failure or success is from musl. The way
as_execvpe() is also inspired by musl (for example, the list of error
codes that should make it fail is the same as in musl's code).
This commit is contained in:
wm4 2020-05-15 16:13:28 +02:00
parent a58b2df3f8
commit 5309497727
1 changed files with 118 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

135
osdep/subprocess-posix.c
View File

@ -15,7 +15,6 @@
* License along with mpv. If not, see <http://www.gnu.org/licenses/>.
*/
#include "osdep/posix-spawn.h"
#include <poll.h>
#include <pthread.h>
#include <unistd.h>
@ -36,18 +35,128 @@ extern char **environ;
#define SAFE_CLOSE(fd) do { if ((fd) >= 0) close((fd)); (fd) = -1; } while (0)
// Async-signal-safe execvpe(). POSIX does not list it as async-signal-safe
// (POSIX is such a joke), so do it manually. While in theory the searching is
// apparently implementation dependent and not exposed (because POSIX is a
// joke?), the expected rules are still relatively simple.
// Doesn't set errno correctly.
// Somewhat inspired by musl's src/process/execvp.c.
static int as_execvpe(const char *path, const char *file, char *const argv[],
char *const envp[])
{
if (strchr(file, '/') || !file[0])
return execve(file, argv, envp);
size_t flen = strlen(file);
while (path && path[0]) {
size_t plen = strcspn(path, ":");
// Ignore paths that are too long.
char fn[PATH_MAX];
if (plen + 1 + flen + 1 < sizeof(fn)) {
memcpy(fn, path, plen);
fn[plen] = '/';
memcpy(fn + plen + 1, file, flen + 1);
execve(fn, argv, envp);
if (errno != EACCES && errno != ENOENT && errno != ENOTDIR)
break;
}
path += plen + (path[plen] == ':' ? 1 : 0);
}
return -1;
}
// Returns 0 on any error, valid PID on success.
// This function must be async-signal-safe, as it may be called from a fork().
static pid_t spawn_process(const char *path, struct mp_subprocess_opts *opts,
int src_fds[])
{
int p[2] = {-1, -1};
pid_t fres = 0;
sigset_t sigmask, oldmask;
sigfillset(&sigmask);
pthread_sigmask(SIG_BLOCK, &sigmask, &oldmask);
// We setup a communication pipe to signal failure. Since the child calls
// exec() and becomes the calling process, we don't know if or when the
// child process successfully ran exec() just from the PID.
// Use a CLOEXEC pipe to detect whether exec() was used. Obviously it will
// be closed if exec() succeeds, and an error is written if not.
// There are also some things further below in the code that need CLOEXEC.
if (mp_make_cloexec_pipe(p) < 0)
goto done;
// Check whether CLOEXEC is really set. Important for correct operation.
int p_flags = fcntl(p[0], F_GETFD);
if (p_flags == -1 || !FD_CLOEXEC || !(p_flags & FD_CLOEXEC))
goto done; // require CLOEXEC; unknown if fallback would be worth it
fres = fork();
if (fres < 0) {
fres = 0;
goto done;
}
if (fres == 0) {
// child
for (int n = 0; n < opts->num_fds; n++) {
if (src_fds[n] == opts->fds[n].fd) {
int flags = fcntl(opts->fds[n].fd, F_GETFD);
if (flags == -1)
goto child_failed;
flags &= ~(unsigned)FD_CLOEXEC;
if (fcntl(opts->fds[n].fd, F_SETFD, flags) == -1)
goto child_failed;
} else if (dup2(src_fds[n], opts->fds[n].fd) < 0) {
goto child_failed;
}
}
as_execvpe(path, opts->exe, opts->args, opts->env ? opts->env : environ);
child_failed:
write(p[1], &(char){1}, 1); // shouldn't be able to fail
_exit(1);
}
SAFE_CLOSE(p[1]);
int r;
do {
r = read(p[0], &(char){0}, 1);
} while (r < 0 && errno == EINTR);
// If exec()ing child failed, collect it immediately.
if (r != 0) {
while (waitpid(fres, &(int){0}, 0) < 0 && errno == EINTR) {}
fres = 0;
}
done:
pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
SAFE_CLOSE(p[0]);
SAFE_CLOSE(p[1]);
return fres;
}
void mp_subprocess2(struct mp_subprocess_opts *opts,
struct mp_subprocess_result *res)
{
posix_spawn_file_actions_t fa;
bool fa_destroy = false;
int status = -1;
int comm_pipe[MP_SUBPROCESS_MAX_FDS][2];
int src_fds[MP_SUBPROCESS_MAX_FDS];
int devnull = -1;
pid_t pid = 0;
bool spawned = false;
bool killed_by_us = false;
int cancel_fd = -1;
char *path = getenv("PATH");
char path_storage[PATH_MAX];
if (!path) {
path = path_storage;
size_t r = confstr(_CS_PATH, path, sizeof(path_storage));
if (r == 0 || r >= sizeof(path_storage))
path[0] = '\0'; // failure, who cares
}
*res = (struct mp_subprocess_result){0};
@ -69,10 +178,6 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
if (devnull < 0)
goto done;
if (posix_spawn_file_actions_init(&fa))
goto done;
fa_destroy = true;
// redirect FDs
for (int n = 0; n < opts->num_fds; n++) {
int src_fd = devnull;
@ -80,12 +185,9 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
src_fd = comm_pipe[n][1];
if (opts->fds[n].src_fd >= 0)
src_fd = opts->fds[n].src_fd;
if (posix_spawn_file_actions_adddup2(&fa, src_fd, opts->fds[n].fd))
goto done;
src_fds[n] = src_fd;
}
char **env = opts->env ? opts->env : environ;
if (opts->detach) {
// If we run it detached, we fork a child to start the process; then
// it exits immediately, letting PID 1 inherit it. So we don't need
@ -99,7 +201,7 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
if (fres == 0) {
// child
setsid();
if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env))
if (!spawn_process(path, opts, src_fds))
_exit(1);
_exit(0);
}
@ -108,13 +210,14 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
while (waitpid(fres, &child_status, 0) < 0 && errno == EINTR) {}
if (!WIFEXITED(child_status) || WEXITSTATUS(child_status) != 0)
goto done;
spawned = true;
} else {
if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env))
pid = spawn_process(path, opts, src_fds);
if (!pid)
goto done;
spawned = true;
}
spawned = true;
for (int n = 0; n < opts->num_fds; n++)
SAFE_CLOSE(comm_pipe[n][1]);
SAFE_CLOSE(devnull);
@ -174,8 +277,6 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
while (waitpid(pid, &status, 0) < 0 && errno == EINTR) {}
done:
if (fa_destroy)
posix_spawn_file_actions_destroy(&fa);
for (int n = 0; n < opts->num_fds; n++) {
SAFE_CLOSE(comm_pipe[n][0]);
SAFE_CLOSE(comm_pipe[n][1]);