Dear colleagues,

checking for hanged and CPU-eating clamav, my colleague ([EMAIL PROTECTED]) found that popen(3) is not thread safe. What about commiting tjr's fix (attached) to RELENG_4?

Sincerely,
D.Marck                                        [DM5020, MCK-RIPE, DM3-RIPN]
---------------------------------------------------------------------------
*** Dmitry Morozovsky --- D.Marck --- Wild Woozle --- [EMAIL PROTECTED] ***
---------------------------------------------------------------------------
Index: lib/libc/gen/popen.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/gen/popen.c,v
retrieving revision 1.14
diff -u -r1.14 popen.c
--- lib/libc/gen/popen.c        27 Jan 2000 23:06:19 -0000      1.14
+++ lib/libc/gen/popen.c        16 Oct 2004 16:14:38 -0000
@@ -51,6 +51,17 @@
 #include <string.h>
 #include <paths.h>
 
+#ifndef _THREAD_SAFE
+#define THREAD_LOCK()
+#define THREAD_UNLOCK()
+#else
+#include <pthread.h>
+#include "libc_private.h"
+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)
+#endif /* _THREAD_SAFE */
+
 extern char **environ;
 
 static struct pid {
@@ -95,8 +106,10 @@
        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);
@@ -134,6 +147,7 @@
                _exit(127);
                /* NOTREACHED */
        }
+       THREAD_UNLOCK();
 
        /* Parent; assume fdopen can't fail. */
        if (*type == 'r') {
@@ -146,9 +160,11 @@
 
        /* 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);
 }
@@ -167,12 +183,23 @@
        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);
+       }
+       /* Remove the entry from the linked list. */
+       if (last == NULL)
+               pidlist = cur->next;
+       else
+               last->next = cur->next;
+       THREAD_UNLOCK();
 
        (void)fclose(iop);
 
@@ -180,11 +207,6 @@
                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);
_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to