Protect pidlist with a mutex to avoid a race causing a duplicate free()
when the same pipe FILE is pclosed()'d in different threads, and to avoid corrupting the linked list when adding or removing items. The symptoms of the linked list getting corrupted were pclose() either not finding the pipe on the list, or the list becoming circular and pclose() looping infinitely.
This commit is contained in:
parent
f8020ddefe
commit
77e2381a3e
@ -51,7 +51,9 @@ __FBSDID("$FreeBSD$");
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <paths.h>
|
||||
#include <pthread.h>
|
||||
#include "un-namespace.h"
|
||||
#include "libc_private.h"
|
||||
|
||||
extern char **environ;
|
||||
|
||||
@ -60,6 +62,10 @@ static struct pid {
|
||||
FILE *fp;
|
||||
pid_t pid;
|
||||
} *pidlist;
|
||||
static pthread_mutex_t pidlist_mutex = PTHREAD_MUTEX_INITIALIZER;
|
||||
|
||||
#define THREAD_LOCK() if (__isthreaded) _pthread_mutex_lock(&pidlist_mutex)
|
||||
#define THREAD_UNLOCK() if (__isthreaded) _pthread_mutex_unlock(&pidlist_mutex)
|
||||
|
||||
FILE *
|
||||
popen(command, type)
|
||||
@ -97,8 +103,10 @@ popen(command, type)
|
||||
argv[2] = (char *)command;
|
||||
argv[3] = NULL;
|
||||
|
||||
THREAD_LOCK();
|
||||
switch (pid = vfork()) {
|
||||
case -1: /* Error. */
|
||||
THREAD_UNLOCK();
|
||||
(void)_close(pdes[0]);
|
||||
(void)_close(pdes[1]);
|
||||
free(cur);
|
||||
@ -136,6 +144,7 @@ popen(command, type)
|
||||
_exit(127);
|
||||
/* NOTREACHED */
|
||||
}
|
||||
THREAD_UNLOCK();
|
||||
|
||||
/* Parent; assume fdopen can't fail. */
|
||||
if (*type == 'r') {
|
||||
@ -148,9 +157,11 @@ popen(command, type)
|
||||
|
||||
/* Link into list of file descriptors. */
|
||||
cur->fp = iop;
|
||||
cur->pid = pid;
|
||||
cur->pid = pid;
|
||||
THREAD_LOCK();
|
||||
cur->next = pidlist;
|
||||
pidlist = cur;
|
||||
THREAD_UNLOCK();
|
||||
|
||||
return (iop);
|
||||
}
|
||||
@ -169,12 +180,22 @@ pclose(iop)
|
||||
int pstat;
|
||||
pid_t pid;
|
||||
|
||||
/* Find the appropriate file pointer. */
|
||||
/*
|
||||
* Find the appropriate file pointer and remove it from the list.
|
||||
*/
|
||||
THREAD_LOCK();
|
||||
for (last = NULL, cur = pidlist; cur; last = cur, cur = cur->next)
|
||||
if (cur->fp == iop)
|
||||
break;
|
||||
if (cur == NULL)
|
||||
if (cur == NULL) {
|
||||
THREAD_UNLOCK();
|
||||
return (-1);
|
||||
}
|
||||
if (last == NULL)
|
||||
pidlist = cur->next;
|
||||
else
|
||||
last->next = cur->next;
|
||||
THREAD_UNLOCK();
|
||||
|
||||
(void)fclose(iop);
|
||||
|
||||
@ -182,11 +203,6 @@ pclose(iop)
|
||||
pid = _wait4(cur->pid, &pstat, 0, (struct rusage *)0);
|
||||
} while (pid == -1 && errno == EINTR);
|
||||
|
||||
/* Remove the entry from the linked list. */
|
||||
if (last == NULL)
|
||||
pidlist = cur->next;
|
||||
else
|
||||
last->next = cur->next;
|
||||
free(cur);
|
||||
|
||||
return (pid == -1 ? -1 : pstat);
|
||||
|
Loading…
x
Reference in New Issue
Block a user