generic-library/bionic
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

libc: popen: work around data corruption

Browse Source 
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
This commit is contained in:
JP Abgrall 2011-09-17 15:22:21 -07:00
parent b8ef90d679
commit 3884bfe966
1 changed files with 11 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
libc/unistd/popen.c
View File

@ -48,6 +48,8 @@ static struct pid {
pid_t pid;
} *pidlist;
extern char **environ;
FILE *
popen(const char *program, const char *type)
{
@ -55,6 +57,7 @@ popen(const char *program, const char *type)
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 @@ popen(const char *program, const char *type)
return (NULL);
}
switch (pid = vfork()) {
switch (pid = fork()) {
case -1: /* Error. */
(void)close(pdes[0]);
(void)close(pdes[1]);
@ -80,24 +83,17 @@ popen(const char *program, const char *type)
{
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 @@ popen(const char *program, const char *type)
(void)close(pdes[0]);
}
}
execl(_PATH_BSHELL, "sh", "-c", program, (char *)NULL);
argp[2] = (char *)program;
execve(_PATH_BSHELL, argp, environ);
_exit(127);
/* NOTREACHED */
}