Rewrite system(3) to use posix_spawn(3).
We saw crashes from pthread_exit+debuggerd on LP32
(https://issuetracker.google.com/72291624), and it seems like the
equivalent problem should exist with system(3). I fixed posix_spawn(3)
as part of that bug, so the easiest fix is probably to reuse that.
Bug: http://b/72470344
Test: ran tests
Change-Id: I05f838706f2b4a14ac3ee21292833e6c8579b0d4
diff --git a/libc/Android.bp b/libc/Android.bp
index 7454d03..20df52c 100644
--- a/libc/Android.bp
+++ b/libc/Android.bp
@@ -481,7 +481,6 @@
"upstream-openbsd/lib/libc/stdlib/reallocarray.c",
"upstream-openbsd/lib/libc/stdlib/remque.c",
"upstream-openbsd/lib/libc/stdlib/setenv.c",
- "upstream-openbsd/lib/libc/stdlib/system.c",
"upstream-openbsd/lib/libc/stdlib/tfind.c",
"upstream-openbsd/lib/libc/stdlib/tsearch.c",
"upstream-openbsd/lib/libc/string/strcasecmp.c",
@@ -1369,6 +1368,7 @@
"bionic/sys_time.cpp",
"bionic/sysinfo.cpp",
"bionic/syslog.cpp",
+ "bionic/system.cpp",
"bionic/system_property_api.cpp",
"bionic/system_property_set.cpp",
"bionic/tdestroy.cpp",
diff --git a/libc/bionic/system.cpp b/libc/bionic/system.cpp
new file mode 100644
index 0000000..7411ae5
--- /dev/null
+++ b/libc/bionic/system.cpp
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <errno.h>
+#include <paths.h>
+#include <stdlib.h>
+#include <spawn.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "private/ScopedSignalBlocker.h"
+#include "private/ScopedSignalHandler.h"
+
+int system(const char* command) {
+ // "The system() function shall always return non-zero when command is NULL."
+ // http://pubs.opengroup.org/onlinepubs/9699919799/functions/system.html
+ if (command == nullptr) return 1;
+
+ ScopedSignalBlocker sigchld_blocker(SIGCHLD);
+ ScopedSignalHandler sigint_ignorer(SIGINT, SIG_IGN);
+ ScopedSignalHandler sigquit_ignorer(SIGQUIT, SIG_IGN);
+
+ sigset64_t default_mask = {};
+ if (sigint_ignorer.old_action_.sa_handler != SIG_IGN) sigaddset64(&default_mask, SIGINT);
+ if (sigquit_ignorer.old_action_.sa_handler != SIG_IGN) sigaddset64(&default_mask, SIGQUIT);
+
+ static constexpr int flags = POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK;
+ posix_spawnattr_t attributes;
+ if ((errno = posix_spawnattr_init(&attributes))) return -1;
+ if ((errno = posix_spawnattr_setsigdefault64(&attributes, &default_mask))) return -1;
+ if ((errno = posix_spawnattr_setsigmask64(&attributes, &sigchld_blocker.old_set_))) return -1;
+ if ((errno = posix_spawnattr_setflags(&attributes, flags))) return -1;
+
+ const char* argv[] = { "sh", "-c", command, nullptr };
+ pid_t child;
+ if ((errno = posix_spawn(&child, _PATH_BSHELL, nullptr, &attributes,
+ const_cast<char**>(argv), environ)) != 0) {
+ return -1;
+ }
+
+ posix_spawnattr_destroy(&attributes);
+
+ int status;
+ pid_t pid = TEMP_FAILURE_RETRY(waitpid(child, &status, 0));
+ return (pid == -1 ? -1 : status);
+}
diff --git a/libc/private/ScopedSignalBlocker.h b/libc/private/ScopedSignalBlocker.h
index 7582068..d1cf629 100644
--- a/libc/private/ScopedSignalBlocker.h
+++ b/libc/private/ScopedSignalBlocker.h
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#ifndef SCOPED_SIGNAL_BLOCKER_H
-#define SCOPED_SIGNAL_BLOCKER_H
+#pragma once
#include <signal.h>
@@ -23,10 +22,18 @@
class ScopedSignalBlocker {
public:
+ // Block all signals.
explicit ScopedSignalBlocker() {
sigset64_t set;
sigfillset64(&set);
- sigprocmask64(SIG_SETMASK, &set, &old_set_);
+ sigprocmask64(SIG_BLOCK, &set, &old_set_);
+ }
+
+ // Block just the specified signal.
+ explicit ScopedSignalBlocker(int signal) {
+ sigset64_t set = {};
+ sigaddset64(&set, signal);
+ sigprocmask64(SIG_BLOCK, &set, &old_set_);
}
~ScopedSignalBlocker() {
@@ -37,10 +44,7 @@
sigprocmask64(SIG_SETMASK, &old_set_, nullptr);
}
- private:
sigset64_t old_set_;
DISALLOW_COPY_AND_ASSIGN(ScopedSignalBlocker);
};
-
-#endif
diff --git a/libc/private/ScopedSignalHandler.h b/libc/private/ScopedSignalHandler.h
new file mode 100644
index 0000000..dd5823f
--- /dev/null
+++ b/libc/private/ScopedSignalHandler.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <signal.h>
+
+class ScopedSignalHandler {
+ public:
+ ScopedSignalHandler(int signal_number, void (*handler)(int), int sa_flags = 0)
+ : signal_number_(signal_number) {
+ action_ = { .sa_flags = sa_flags, .sa_handler = handler };
+ sigaction64(signal_number_, &action_, &old_action_);
+ }
+
+ ScopedSignalHandler(int signal_number, void (*action)(int, siginfo_t*, void*),
+ int sa_flags = SA_SIGINFO)
+ : signal_number_(signal_number) {
+ action_ = { .sa_flags = sa_flags, .sa_sigaction = action };
+ sigaction64(signal_number_, &action_, &old_action_);
+ }
+
+ ScopedSignalHandler(int signal_number) : signal_number_(signal_number) {
+ sigaction64(signal_number, nullptr, &old_action_);
+ }
+
+ ~ScopedSignalHandler() {
+ sigaction64(signal_number_, &old_action_, nullptr);
+ }
+
+ struct sigaction64 action_;
+ struct sigaction64 old_action_;
+ const int signal_number_;
+};
diff --git a/libc/upstream-openbsd/lib/libc/stdlib/system.c b/libc/upstream-openbsd/lib/libc/stdlib/system.c
deleted file mode 100644
index de32d43..0000000
--- a/libc/upstream-openbsd/lib/libc/stdlib/system.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/* $OpenBSD: system.c,v 1.11 2015/10/23 04:44:41 guenther Exp $ */
-/*
- * Copyright (c) 1988 The Regents of the University of California.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <errno.h>
-#include <signal.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <paths.h>
-
-extern char **environ;
-
-int
-system(const char *command)
-{
- pid_t pid, cpid;
- struct sigaction intsave, quitsave;
- sigset_t mask, omask;
- int pstat;
- char *argp[] = {"sh", "-c", NULL, NULL};
-
- if (!command) /* just checking... */
- return(1);
-
- argp[2] = (char *)command;
-
- sigemptyset(&mask);
- sigaddset(&mask, SIGCHLD);
- sigprocmask(SIG_BLOCK, &mask, &omask);
- switch (cpid = vfork()) {
- case -1: /* error */
- sigprocmask(SIG_SETMASK, &omask, NULL);
- return(-1);
- case 0: /* child */
- sigprocmask(SIG_SETMASK, &omask, NULL);
- execve(_PATH_BSHELL, argp, environ);
- _exit(127);
- }
-
- sigaction(SIGINT, NULL, &intsave);
- sigaction(SIGQUIT, NULL, &quitsave);
- do {
- pid = waitpid(cpid, &pstat, 0);
- } while (pid == -1 && errno == EINTR);
- sigprocmask(SIG_SETMASK, &omask, NULL);
- sigaction(SIGINT, &intsave, NULL);
- sigaction(SIGQUIT, &quitsave, NULL);
- return (pid == -1 ? -1 : pstat);
-}
-DEF_STRONG(system);