tags 575019 + patch
thanks
I took a brief look at the code and the changes introduced in
0.3.1-8+lenny1 in particular. The problem is that two pairs of
popen/pclose calls have been replaced by fork+fdopen/fclose. This
removes the implicit wait4 done by pclose.
I'm afraid spamass-milter does wait for other children here and there,
which is why your initial fix triggered an error. Ignoring the SIGCHLDs
will cause those other waitpid's to fail.
So we need to add more of a real replacement for pclose. The original
popen will save the pid in a private list. We could have done the same,
but since this code isn't used anywhere else I went for a somewhat
simpler approach. The attached patch on top of 0.3.1-8+lenny1 fixes
this bug for me, and I assume it will fix a similar bug for those using
the -b or -B options (not confirmed).
Bjørn
diff -ur spamass-milter-0.3.1.security-lenny1/spamass-milter.cpp spamass-milter-0.3.1/spamass-milter.cpp
--- spamass-milter-0.3.1.security-lenny1/spamass-milter.cpp 2010-03-23 11:12:07.000000000 +0100
+++ spamass-milter-0.3.1/spamass-milter.cpp 2010-03-23 12:51:39.000000000 +0100
@@ -465,13 +465,14 @@
the only way to do it. */
char *popen_argv[3];
FILE *p;
+ pid_t pid;
popen_argv[0] = SENDMAIL;
popen_argv[1] = spambucket;
popen_argv[2] = NULL;
debug(D_COPY, "calling %s %s", SENDMAIL, spambucket);
- p = popenv(popen_argv, "w");
+ p = popenv(popen_argv, "w", &pid);
if (!p)
{
debug(D_COPY, "popenv failed(%s). Will not send a copy to spambucket", strerror(errno));
@@ -480,6 +481,7 @@
// Send message provided by SpamAssassin
fwrite(assassin->d().c_str(), assassin->d().size(), 1, p);
fclose(p); p = NULL;
+ waitpid(pid, NULL, 0);
}
}
return SMFIS_REJECT;
@@ -826,6 +828,7 @@
char buf[1024];
char *popen_argv[4];
+ pid_t pid;
popen_argv[0] = SENDMAIL;
popen_argv[1] = "-bv";
@@ -834,7 +837,7 @@
debug(D_RCPT, "calling %s -bv %s", SENDMAIL, envrcpt[0]);
- p = popenv(popen_argv, "r");
+ p = popenv(popen_argv, "r", &pid);
if (!p)
{
debug(D_RCPT, "popenv failed(%s). Will not expand aliases", strerror(errno));
@@ -863,6 +866,7 @@
}
}
fclose(p); p = NULL;
+ waitpid(pid, NULL, 0);
}
} else
{
@@ -2126,11 +2130,12 @@
for simplicity, and always reads stdout and stderr in "r" mode. Call
fclose to close the FILE.
*/
-FILE *popenv(char *const argv[], const char *type)
+FILE *popenv(char *const argv[], const char *type, pid_t *pid)
{
FILE *iop;
int pdes[2];
int save_errno;
+ pid_t tmpid;
if ((*type != 'r' && *type != 'w') || type[1])
{
errno = EINVAL;
@@ -2138,7 +2143,7 @@
}
if (pipe(pdes) < 0)
return (NULL);
- switch (fork()) {
+ switch (tmpid = fork()) {
case -1: /* Error. */
save_errno = errno;
@@ -2184,6 +2189,10 @@
(void)close(pdes[0]);
}
+ /* return pid so that we can wait for it */
+ if (pid)
+ *pid = tmpid;
+
return (iop);
}
diff -ur spamass-milter-0.3.1.security-lenny1/spamass-milter.h spamass-milter-0.3.1/spamass-milter.h
--- spamass-milter-0.3.1.security-lenny1/spamass-milter.h 2010-03-23 11:12:07.000000000 +0100
+++ spamass-milter-0.3.1/spamass-milter.h 2010-03-23 12:52:10.000000000 +0100
@@ -186,6 +186,6 @@
void parse_debuglevel(char* string);
char *strlwr(char *str);
void warnmacro(char *macro, char *scope);
-FILE *popenv(char *const argv[], const char *type);
+FILE *popenv(char *const argv[], const char *type, pid_t *pid);
#endif
