pkg/44704: spamass-milter livelock (patch)

[hannken%eis.cs.tu-bs.de@localhost]

>Number:         44704
>Category:       pkg
>Synopsis:       spamass-milter livelock (patch)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Mar 09 16:35:00 +0000 2011
>Originator:     Juergen Hannken-Illjes
>Release:        NetBSD 5.0.2 / pkgsrc-2010Q4
>Organization:
        
>Environment:
        
        
System: NetBSD builder 5.0.2 NetBSD 5.0.2 (local.amd64) #1: Mon Feb 7 09:27:25 
MET 2011 
build@builder:/build/nbsd5/obj/obj.amd64/sys/arch/amd64/compile/local.amd64 
amd64
Architecture: x86_64
Machine: amd64
>Description:
For me spamass-milter under heavy load forks threads that livelock.  Looks
like both c++ string operations and malloc in the child after fork() and
before execvp() are bad things in a pthreaded application.

Attached patch resolves the problem for me by moving everything into the
parent and on the stack.
        
>How-To-Repeat:
Put spamass-milter under heavy load and see threads running with 100% cpu.
        
>Fix:
--- spamass-milter.cpp.orig     2011-03-06 14:03:18.000000000 +0100
+++ spamass-milter.cpp
@@ -1300,4 +1300,20 @@ SpamAssassin::~SpamAssassin()
 void SpamAssassin::Connect()
 {
+  int argc;
+  char *argv[100];
+  char spamc_user[64];
+
+  if (expandedrcpt.size() != 1) {
+    debug(D_RCPT, "%d recipients; spamc gets default username %s", 
(int)expandedrcpt.size(), defaultuser);
+    strlcpy(spamc_user, defaultuser, sizeof(spamc_user));
+  } else {
+    if (flag_full_email)
+      strlcpy(spamc_user, full_user().c_str(), sizeof(spamc_user)); 
+    else
+      strlcpy(spamc_user, local_user().c_str(), sizeof(spamc_user)); 
+    strlwr(spamc_user);
+    debug(D_RCPT, "spamc gets %s", spamc_user);
+  }
+
   // set up pipes for in- and output
   if (pipe(pipe_io[0]))
@@ -1333,31 +1349,10 @@ void SpamAssassin::Connect()
       // should be a little more secure
       // XXX arbitrary 100-argument max
-      int argc = 0;
-      char** argv = (char**) malloc(100*sizeof(char*));
+      argc = 0;
       argv[argc++] = SPAMC;
       if (flag_sniffuser) 
       {
         argv[argc++] = "-u";
-        if ( expandedrcpt.size() != 1 )
-        {
-          // More (or less?) than one recipient, so we pass the default
-          // username to SPAMC.  This way special rules can be defined for
-          // multi recipient messages.
-          debug(D_RCPT, "%d recipients; spamc gets default username %s", 
(int)expandedrcpt.size(), defaultuser);
-          argv[argc++] = defaultuser; 
-        } else
-        { 
-          // There is only 1 recipient so we pass the username
-          // (converted to lowercase) to SPAMC.  Don't worry about 
-          // freeing this memory as we're exec()ing anyhow.
-          if (flag_full_email)
-            argv[argc] = strlwr(strdup(full_user().c_str())); 
-          else
-            argv[argc] = strlwr(strdup(local_user().c_str())); 
-
-          debug(D_RCPT, "spamc gets %s", argv[argc]);
-         
-          argc++;
-        }
+        argv[argc++] = spamc_user;
       }
       if (spamdhost) 

>Unformatted: