Skip to content

Commit 753a71d

Browse files
committed
subprocess-posix: fewer ifdefs and fix ASan
1 parent b6dc978 commit 753a71d

1 file changed

Lines changed: 20 additions & 23 deletions

File tree

osdep/subprocess-posix.c

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,11 @@ static int as_execvpe(const char *path, const char *file, char *const argv[],
7474
return -1;
7575
}
7676

77-
#if !HAVE_RFORK
7877
// In the child process, resets the signal mask to defaults. Also clears any
7978
// signal handlers first so nothing funny happens.
8079
static void reset_signals_child(void)
8180
{
81+
#if !HAVE_RFORK
8282
struct sigaction sa = { 0 };
8383
sigset_t sigmask;
8484
sa.sa_handler = SIG_DFL;
@@ -87,29 +87,34 @@ static void reset_signals_child(void)
8787
for (int nr = 1; nr < SIGNAL_MAX; nr++)
8888
sigaction(nr, &sa, NULL);
8989
sigprocmask(SIG_SETMASK, &sigmask, NULL);
90-
}
9190
#endif
91+
}
9292

9393
struct child_args {
9494
const char *path;
9595
struct mp_subprocess_opts *opts;
9696
int *src_fds;
9797
void *child_stack;
9898
int pipe_end;
99+
bool failed;
99100
bool detach;
100101
};
101102

102103
static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *opts,
103104
int src_fds[], bool detach, void *stacks[]);
104105

106+
// This function is called from a clone(CLONE_VM)/rfork_thread context where
107+
// the child shares the parent's address space. Use no_sanitize to avoid false
108+
// positives from ASan when the child writes to shared memory.
109+
__attribute__((no_sanitize("address")))
105110
static int child_main(void* args)
106111
{
107112
struct child_args *child_args = args;
108113
const char *path = child_args->path;
109114
struct mp_subprocess_opts *opts = child_args->opts;
110115
int *src_fds = child_args->src_fds;
111116
void *child_stack = child_args->child_stack;
112-
int *pipe_end = &child_args->pipe_end;
117+
int pipe_end = child_args->pipe_end;
113118
bool detach = child_args->detach;
114119

115120
if (detach) {
@@ -120,9 +125,7 @@ static int child_main(void* args)
120125
}
121126

122127
// RFSPAWN has reset all signal actions in the child to default
123-
#if !HAVE_RFORK
124128
reset_signals_child();
125-
#endif
126129

127130
for (int n = 0; n < opts->num_fds; n++) {
128131
if (src_fds[n] == opts->fds[n].fd) {
@@ -140,35 +143,35 @@ static int child_main(void* args)
140143
as_execvpe(path, opts->exe, opts->args, opts->env ? opts->env : environ);
141144

142145
child_failed:
143-
#if HAVE_CLONE || HAVE_RFORK
144-
*pipe_end = 1;
145-
#else
146-
(void)write(*pipe_end, &(char){1}, 1); // shouldn't be able to fail
147-
#endif
146+
child_args->failed = true;
147+
if (pipe_end >= 0)
148+
(void)write(pipe_end, &(char){1}, 1); // shouldn't be able to fail
148149
return 1;
149150
}
150151

151152
static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *opts,
152153
int src_fds[], bool detach, void *stacks[])
153154
{
154155
pid_t fres = 0;
155-
int r;
156+
int r = 0;
156157

157158
struct child_args child_args = {
158159
.path = path,
159160
.opts = opts,
160161
.src_fds = src_fds,
161162
.child_stack = stacks[1],
162-
.pipe_end = 0,
163+
.pipe_end = -1,
164+
.failed = false,
163165
.detach = detach,
164166
};
165167

168+
int p[2] = {-1, -1};
169+
166170
#if HAVE_RFORK
167171
fres = rfork_thread(RFSPAWN, stacks[0], child_main, &child_args);
168172
#elif HAVE_CLONE
169173
fres = clone(child_main, stacks[0], CLONE_VM | CLONE_VFORK | SIGCHLD, &child_args);
170174
#else
171-
int p[2] = {-1, -1};
172175
// We setup a communication pipe to signal failure. Since the child calls
173176
// exec() and becomes the calling process, we don't know if or when the
174177
// child process successfully ran exec() just from the PID.
@@ -191,9 +194,7 @@ static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *op
191194
goto done;
192195
}
193196

194-
#if HAVE_CLONE || HAVE_RFORK
195-
r = child_args.pipe_end;
196-
#else
197+
#if !HAVE_CLONE && !HAVE_RFORK
197198
if (fres == 0) {
198199
_exit(child_main(&child_args));
199200
}
@@ -206,34 +207,32 @@ static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *op
206207
#endif
207208

208209
// If exec()ing child failed, collect it immediately.
209-
if (detach || r != 0) {
210+
if (detach || child_args.failed || r != 0) {
210211
int child_status = 0;
211212
while (waitpid(fres, &child_status, 0) < 0 && errno == EINTR) {}
212213
if (r != 0 || !WIFEXITED(child_status) || WEXITSTATUS(child_status) != 0)
213214
fres = 0;
214215
}
215216

216217
done:
217-
#if !HAVE_CLONE && !HAVE_RFORK
218218
SAFE_CLOSE(p[0]);
219219
SAFE_CLOSE(p[1]);
220-
#endif
221220

222221
return fres;
223222
}
224223

225224
// Returns 0 on any error, valid PID on success.
226-
// This function must be async-signal-safe, as it may be called from a fork().
227225
static pid_t spawn_process(const char *path, struct mp_subprocess_opts *opts,
228226
int src_fds[])
229227
{
230228
bool detach = opts->detach;
231229
void *stacks[2];
230+
void *ctx = NULL;
232231

233232
#if HAVE_CLONE || HAVE_RFORK
234233
// pre-allocate stacks so we can be async-signal-safe in spawn_process_inner()
235234
const size_t stack_size = 0x8000;
236-
void *ctx = talloc_new(NULL);
235+
ctx = talloc_new(NULL);
237236
// stack should be aligned to 16 bytes, which is guaranteed by malloc
238237
stacks[0] = (int8_t*)talloc_size(ctx, stack_size) + stack_size;
239238
if (detach) stacks[1] = (int8_t*)talloc_size(ctx, stack_size) + stack_size;
@@ -252,9 +251,7 @@ static pid_t spawn_process(const char *path, struct mp_subprocess_opts *opts,
252251
pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
253252
#endif
254253

255-
#if HAVE_CLONE || HAVE_RFORK
256254
talloc_free(ctx);
257-
#endif
258255

259256
return fres;
260257
}

0 commit comments

Comments
 (0)