On Mon, 19 Apr 2004, Mike Bethune wrote:
> maybe why I noticed this starting in 4.24:
> Exim version 4.24
> -----------------
> 6. When the daemon running as "exim" started a queue runner, it always
> re-executed Exim in the spun-off process. This is a waste of effort when
> deliver_drop_privilege is set. The new process now just calls the
> queue-runner function directly.
>
> If deliver_drop_privilege is set and we already dropped privilege (or
> never had any :), then the signal handlers don't get set up since they
> are reset in this child (see below snip from daemon.c). Probably the
> signal handlers should be set up again before the queue_run() or they
> shouldn't be reset here? (I'd like to not have to re-exec since it is
> a waste).
Absolutely correct. Thanks for doing most of the debugging work. I have
now reproduced the problem, and fixed it. The patch is below. The actual
operative part is moving the handler reset into the re-exec case, but I
did some other related tidying. The ChangeLog reads:
1. Change 4.24/6 introduced a bug because the SIGALRM handler was disabled
before starting a queue runner without re-exec. This happened only when
deliver_drop_privilege was set. The effect of the bug was that timeouts
during subsequent deliveries caused crashes instead of being properly
handled. The handler is now left at its default (and expected) setting.
2. The other case in which a daemon avoids a re-exec is to deliver an incoming
message, again when deliver_drop_privilege is set. The bug described in (1)
was not present in this case, but the tidying up of the other signals was
missing. I have made the two cases consistent.
Philip
--
Philip Hazel University of Cambridge Computing Service,
ph10@??? Cambridge, England. Phone: +44 1223 334714.
Get the Exim 4 book: http://www.uit.co.uk/exim-book
*** exim-4.32/src/daemon.c Thu Apr 15 09:27:01 2004
--- daemon.c Tue Apr 20 12:18:20 2004
***************
*** 540,553 ****
tls_close(FALSE);
#endif
if (geteuid() != root_uid && !deliver_drop_privilege)
{
(void)child_exec_exim(CEE_EXEC_PANIC, FALSE, NULL, FALSE, 2, US"-Mc",
message_id);
/* Control does not return here. */
}
! /* No need to re-exec */
(void)deliver_message(message_id, FALSE, FALSE);
search_tidyup();
_exit(EXIT_SUCCESS);
--- 540,560 ----
tls_close(FALSE);
#endif
+ /* Reset SIGHUP and SIGCHLD in the child in both cases. */
+
+ signal(SIGHUP, SIG_DFL);
+ signal(SIGCHLD, SIG_DFL);
+
if (geteuid() != root_uid && !deliver_drop_privilege)
{
+ signal(SIGALRM, SIG_DFL);
(void)child_exec_exim(CEE_EXEC_PANIC, FALSE, NULL, FALSE, 2, US"-Mc",
message_id);
/* Control does not return here. */
}
! /* No need to re-exec; SIGALRM remains set to the default handler */
!
(void)deliver_message(message_id, FALSE, FALSE);
search_tidyup();
_exit(EXIT_SUCCESS);
***************
*** 1418,1437 ****
for (sk = 0; sk < listen_socket_count; sk++) close(listen_sockets[sk]);
! /* Reset signals in the child */
- signal(SIGALRM, SIG_DFL);
signal(SIGHUP, SIG_DFL);
signal(SIGCHLD, SIG_DFL);
/* Re-exec if privilege has been given up, unless deliver_drop_
! privilege is set. */
if (geteuid() != root_uid && !deliver_drop_privilege)
{
uschar opt[8];
uschar *p = opt;
*p++ = '-';
*p++ = 'q';
if (queue_2stage) *p++ = 'q';
--- 1425,1444 ----
for (sk = 0; sk < listen_socket_count; sk++) close(listen_sockets[sk]);
! /* Reset SIGHUP and SIGCHLD in the child in both cases. */
signal(SIGHUP, SIG_DFL);
signal(SIGCHLD, SIG_DFL);
/* Re-exec if privilege has been given up, unless deliver_drop_
! privilege is set. Reset SIGALRM before exec(). */
if (geteuid() != root_uid && !deliver_drop_privilege)
{
uschar opt[8];
uschar *p = opt;
+ signal(SIGALRM, SIG_DFL);
*p++ = '-';
*p++ = 'q';
if (queue_2stage) *p++ = 'q';
***************
*** 1445,1451 ****
/* Control never returns here. */
}
! /* No need to re-exec */
queue_run(NULL, NULL, FALSE);
_exit(EXIT_SUCCESS);
--- 1452,1458 ----
/* Control never returns here. */
}
! /* No need to re-exec; SIGALRM remains set to the default handler */
queue_run(NULL, NULL, FALSE);
_exit(EXIT_SUCCESS);
