libc: popen: work around data corruption
vfork() would not save the registers that the parent would expect
to have restored after execl() completed.
Specially that execl() would call execve() underneath, further messing
up the stack of the parent.
To avoid that, we fork() for now. Later we will revisit and cleanup
vfork()+execve() to actually have vfork() store all the register
that the parent expects to see, and not those left by execve().
In the original code, looking at the registers just before the call to
popen(), and after the call showed that r7 would get clobbered.
This would leave the caller with an invalid pointer, leading to all
kinds of data corruptions.
execve() is simpler that execl() in this case.
Bug: 5336252
Change-Id: I3bf718c0bb4c0439f6f2753f153cdea14175be9c
diff --git a/libc/unistd/popen.c b/libc/unistd/popen.c
index 15f8325..c2355c1 100644
--- a/libc/unistd/popen.c
+++ b/libc/unistd/popen.c
@@ -48,6 +48,8 @@
pid_t pid;
} *pidlist;
+extern char **environ;
+
FILE *
popen(const char *program, const char *type)
{
@@ -55,6 +57,7 @@
FILE *iop;
int pdes[2];
pid_t pid;
+ char *argp[] = {"sh", "-c", NULL, NULL};
if ((*type != 'r' && *type != 'w') || type[1] != '\0') {
errno = EINVAL;
@@ -69,7 +72,7 @@
return (NULL);
}
- switch (pid = vfork()) {
+ switch (pid = fork()) {
case -1: /* Error. */
(void)close(pdes[0]);
(void)close(pdes[1]);
@@ -80,24 +83,17 @@
{
struct pid *pcur;
/*
- * because vfork() instead of fork(), must leak FILE *,
- * but luckily we are terminally headed for an execl()
+ * We fork()'d, we got our own copy of the list, no
+ * contention.
*/
for (pcur = pidlist; pcur; pcur = pcur->next)
close(fileno(pcur->fp));
if (*type == 'r') {
- int tpdes1 = pdes[1];
-
(void) close(pdes[0]);
- /*
- * We must NOT modify pdes, due to the
- * semantics of vfork.
- */
- if (tpdes1 != STDOUT_FILENO) {
- (void)dup2(tpdes1, STDOUT_FILENO);
- (void)close(tpdes1);
- tpdes1 = STDOUT_FILENO;
+ if (pdes[1] != STDOUT_FILENO) {
+ (void)dup2(pdes[1], STDOUT_FILENO);
+ (void)close(pdes[1]);
}
} else {
(void)close(pdes[1]);
@@ -106,7 +102,8 @@
(void)close(pdes[0]);
}
}
- execl(_PATH_BSHELL, "sh", "-c", program, (char *)NULL);
+ argp[2] = (char *)program;
+ execve(_PATH_BSHELL, argp, environ);
_exit(127);
/* NOTREACHED */
}
