Commit 18de516a 18de516a00c01a4262310f09a1ac8586e97f8a1a by Sergey Poznyakoff

imap4d: redo signal handling

Previously implemented way of signal handling was unsafe because of
the use of unsafe functions in signal handlers. It also allowed for
recursive invocations of MU calls not supposed to handle recursion
(such as mu_mailbox_expunge, for example). This changeset fixes it.

* imap4d/imap4d.c (imap4d_child_signal_setup): Change signal set.
(imap4d_mainloop): Set a jump point for signal handling.
Restore default handling for SIGILL, SIGBUS, SIGFPE, SIGSEGV, SIGSTOP.
(master_jmp): New variable.
(imap4d_master_signal): New function.
(main): Redo signal handling.
* imap4d/imap4d.h (child_jmp): New extern.
(imap4d_enter_critical,imap4d_leave_critical): New protos.
* imap4d/signal.c (imap4d_master_signal): Move to imap4d.c
(imap4d_enter_critical,imap4d_leave_critical): New functions.
(imap4d_child_signal): Rewrite.

* imap4d/append.c: Protect critical sections.
* imap4d/bye.c: Likewise.
* imap4d/close.c: Likewise.
* imap4d/copy.c: Likewise.
* imap4d/delete.c: Likewise.
* imap4d/expunge.c: Likewise.
* imap4d/rename.c: Likewise.
* imap4d/select.c: Likewise.
* imap4d/status.c: Likewise.

* scheme/Makefile.am (sievemod_DATA): Add guimb.scmi.
1 parent c7376cec
...@@ -125,6 +125,7 @@ imap4d_append0 (mu_mailbox_t mbox, int flags, char *date_time, char *text, ...@@ -125,6 +125,7 @@ imap4d_append0 (mu_mailbox_t mbox, int flags, char *date_time, char *text,
125 return 1; 125 return 1;
126 } 126 }
127 127
128 imap4d_enter_critical ();
128 rc = mu_mailbox_append_message (mbox, msg); 129 rc = mu_mailbox_append_message (mbox, msg);
129 if (rc == 0) 130 if (rc == 0)
130 { 131 {
...@@ -140,6 +141,7 @@ imap4d_append0 (mu_mailbox_t mbox, int flags, char *date_time, char *text, ...@@ -140,6 +141,7 @@ imap4d_append0 (mu_mailbox_t mbox, int flags, char *date_time, char *text,
140 /* FIXME: If not INBOX */ 141 /* FIXME: If not INBOX */
141 quota_update (size); 142 quota_update (size);
142 } 143 }
144 imap4d_leave_critical ();
143 145
144 mu_message_destroy (&msg, &tm); 146 mu_message_destroy (&msg, &tm);
145 return rc; 147 return rc;
......
...@@ -36,17 +36,39 @@ imap4d_bye0 (int reason, struct imap4d_command *command) ...@@ -36,17 +36,39 @@ imap4d_bye0 (int reason, struct imap4d_command *command)
36 { 36 {
37 int status = EX_SOFTWARE; 37 int status = EX_SOFTWARE;
38 38
39 /* Some clients may close the connection immediately after sending
40 LOGOUT. Do not treat this as error (RFC 2683).
41 To be on the safe side, ignore broken pipes for any command: at
42 this stage it is of no significance. */
43 static int sigtab[] = { SIGPIPE };
44 mu_set_signals (sigpipe, sigtab, MU_ARRAY_SIZE (sigtab));
45 if (setjmp (pipejmp))
46 {
47 mu_set_signals (SIG_IGN, sigtab, MU_ARRAY_SIZE (sigtab));
48 /* Invalidate iostream. This creates a mild memory leak, as the
49 stream is not destroyed by util_bye, but at least this avoids
50 endless loop which would happen when mu_stream_flush would
51 try to flush buffers on an already broken pipe.
52 FIXME: There must be a special function for that, I guess.
53 Something like mu_stream_invalidate. */
54 iostream = NULL;
55 }
56 else
57 {
39 if (mbox) 58 if (mbox)
40 { 59 {
60 imap4d_enter_critical ();
41 mu_mailbox_flush (mbox, 0); 61 mu_mailbox_flush (mbox, 0);
42 mu_mailbox_close (mbox); 62 mu_mailbox_close (mbox);
43 mu_mailbox_destroy (&mbox); 63 mu_mailbox_destroy (&mbox);
64 imap4d_leave_critical ();
44 } 65 }
45 66
46 switch (reason) 67 switch (reason)
47 { 68 {
48 case ERR_NO_MEM: 69 case ERR_NO_MEM:
49 io_untagged_response (RESP_BYE, "Server terminating: no more resources."); 70 io_untagged_response (RESP_BYE,
71 "Server terminating: no more resources.");
50 mu_diag_output (MU_DIAG_ERROR, _("not enough memory")); 72 mu_diag_output (MU_DIAG_ERROR, _("not enough memory"));
51 break; 73 break;
52 74
...@@ -66,7 +88,8 @@ imap4d_bye0 (int reason, struct imap4d_command *command) ...@@ -66,7 +88,8 @@ imap4d_bye0 (int reason, struct imap4d_command *command)
66 if (state == STATE_NONAUTH) 88 if (state == STATE_NONAUTH)
67 mu_diag_output (MU_DIAG_INFO, _("session timed out for no user")); 89 mu_diag_output (MU_DIAG_INFO, _("session timed out for no user"));
68 else 90 else
69 mu_diag_output (MU_DIAG_INFO, _("session timed out for user: %s"), auth_data->name); 91 mu_diag_output (MU_DIAG_INFO, _("session timed out for user: %s"),
92 auth_data->name);
70 break; 93 break;
71 94
72 case ERR_NO_OFILE: 95 case ERR_NO_OFILE:
...@@ -95,31 +118,20 @@ imap4d_bye0 (int reason, struct imap4d_command *command) ...@@ -95,31 +118,20 @@ imap4d_bye0 (int reason, struct imap4d_command *command)
95 if (state == STATE_NONAUTH) 118 if (state == STATE_NONAUTH)
96 mu_diag_output (MU_DIAG_INFO, _("session terminating")); 119 mu_diag_output (MU_DIAG_INFO, _("session terminating"));
97 else 120 else
98 mu_diag_output (MU_DIAG_INFO, _("session terminating for user: %s"), auth_data->name); 121 mu_diag_output (MU_DIAG_INFO,
122 _("session terminating for user: %s"),
123 auth_data->name);
99 break; 124 break;
100 125
101 default: 126 default:
102 io_untagged_response (RESP_BYE, "Quitting (reason unknown)"); 127 io_untagged_response (RESP_BYE, "Quitting (reason unknown)");
103 mu_diag_output (MU_DIAG_ERROR, _("quitting (numeric reason %d)"), reason); 128 mu_diag_output (MU_DIAG_ERROR, _("quitting (numeric reason %d)"),
129 reason);
104 break; 130 break;
105 } 131 }
106 132
107 if (status == EX_OK && command) 133 if (status == EX_OK && command)
108 {
109 /* Some clients may close the connection immediately after sending
110 LOGOUT. Do not treat this as error (RFC 2683). */
111 static int sigtab[] = { SIGPIPE };
112 mu_set_signals (sigpipe, sigtab, MU_ARRAY_SIZE (sigtab));
113 if (setjmp (pipejmp) == 0)
114 io_completion_response (command, RESP_OK, "Completed"); 134 io_completion_response (command, RESP_OK, "Completed");
115 else
116 /* Invalidate iostream. This creates a mild memory leak, as the
117 stream is not destroyed by util_bye, but at least this avoids
118 endless loop which would happen when mu_stream_flush would
119 try to flush buffers on an already broken pipe.
120 FIXME: There must be a special function for that, I guess.
121 Something like mu_stream_invalidate. */
122 iostream = NULL;
123 } 135 }
124 136
125 util_bye (); 137 util_bye ();
......
...@@ -30,7 +30,9 @@ imap4d_close0 (struct imap4d_command *command, imap4d_tokbuf_t tok, ...@@ -30,7 +30,9 @@ imap4d_close0 (struct imap4d_command *command, imap4d_tokbuf_t tok,
30 mu_mailbox_get_flags (mbox, &flags); 30 mu_mailbox_get_flags (mbox, &flags);
31 if (flags & MU_STREAM_WRITE) 31 if (flags & MU_STREAM_WRITE)
32 { 32 {
33 imap4d_enter_critical ();
33 status = mu_mailbox_flush (mbox, expunge); 34 status = mu_mailbox_flush (mbox, expunge);
35 imap4d_leave_critical ();
34 if (status) 36 if (status)
35 { 37 {
36 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_flush", NULL, status); 38 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_flush", NULL, status);
...@@ -40,7 +42,9 @@ imap4d_close0 (struct imap4d_command *command, imap4d_tokbuf_t tok, ...@@ -40,7 +42,9 @@ imap4d_close0 (struct imap4d_command *command, imap4d_tokbuf_t tok,
40 42
41 /* No messages are removed, and no error is given, if the mailbox is 43 /* No messages are removed, and no error is given, if the mailbox is
42 selected by an EXAMINE command or is otherwise selected read-only. */ 44 selected by an EXAMINE command or is otherwise selected read-only. */
45 imap4d_enter_critical ();
43 status = mu_mailbox_close (mbox); 46 status = mu_mailbox_close (mbox);
47 imap4d_leave_critical ();
44 if (status) 48 if (status)
45 { 49 {
46 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_close", NULL, status); 50 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_close", NULL, status);
......
...@@ -119,7 +119,9 @@ try_copy (mu_mailbox_t dst, mu_mailbox_t src, size_t n, size_t *set) ...@@ -119,7 +119,9 @@ try_copy (mu_mailbox_t dst, mu_mailbox_t src, size_t n, size_t *set)
119 return RESP_BAD; 119 return RESP_BAD;
120 } 120 }
121 121
122 imap4d_enter_critical ();
122 status = mu_mailbox_append_message (dst, msg); 123 status = mu_mailbox_append_message (dst, msg);
124 imap4d_leave_critical ();
123 if (status) 125 if (status)
124 { 126 {
125 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_append_message", 127 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_append_message",
...@@ -186,7 +188,9 @@ safe_copy (mu_mailbox_t dst, mu_mailbox_t src, size_t n, size_t *set, ...@@ -186,7 +188,9 @@ safe_copy (mu_mailbox_t dst, mu_mailbox_t src, size_t n, size_t *set,
186 } 188 }
187 } 189 }
188 190
191 imap4d_enter_critical ();
189 status = mu_mailbox_flush (dst, 1); 192 status = mu_mailbox_flush (dst, 1);
193 imap4d_leave_critical ();
190 if (status) 194 if (status)
191 { 195 {
192 mu_url_t url = NULL; 196 mu_url_t url = NULL;
......
...@@ -56,7 +56,9 @@ imap4d_delete (struct imap4d_command *command, imap4d_tokbuf_t tok) ...@@ -56,7 +56,9 @@ imap4d_delete (struct imap4d_command *command, imap4d_tokbuf_t tok)
56 rc = mu_mailbox_create (&tmpbox, name); 56 rc = mu_mailbox_create (&tmpbox, name);
57 if (rc == 0) 57 if (rc == 0)
58 { 58 {
59 imap4d_enter_critical ();
59 rc = mu_mailbox_remove (tmpbox); 60 rc = mu_mailbox_remove (tmpbox);
61 imap4d_leave_critical ();
60 mu_mailbox_destroy (&tmpbox); 62 mu_mailbox_destroy (&tmpbox);
61 if (rc) 63 if (rc)
62 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_remove", name, rc); 64 mu_diag_funcall (MU_DIAG_ERROR, "mu_mailbox_remove", name, rc);
......
...@@ -36,8 +36,10 @@ imap4d_expunge (struct imap4d_command *command, imap4d_tokbuf_t tok) ...@@ -36,8 +36,10 @@ imap4d_expunge (struct imap4d_command *command, imap4d_tokbuf_t tok)
36 if (imap4d_tokbuf_argc (tok) != 2) 36 if (imap4d_tokbuf_argc (tok) != 2)
37 return io_completion_response (command, RESP_BAD, "Invalid arguments"); 37 return io_completion_response (command, RESP_BAD, "Invalid arguments");
38 38
39 imap4d_enter_critical ();
39 /* FIXME: check for errors. */ 40 /* FIXME: check for errors. */
40 mu_mailbox_expunge (mbox); 41 mu_mailbox_expunge (mbox);
42 imap4d_leave_critical ();
41 43
42 imap4d_sync_invalidate (); 44 imap4d_sync_invalidate ();
43 imap4d_sync (); 45 imap4d_sync ();
......
...@@ -385,11 +385,12 @@ get_client_address (int fd, struct sockaddr_in *pcs) ...@@ -385,11 +385,12 @@ get_client_address (int fd, struct sockaddr_in *pcs)
385 return 0; 385 return 0;
386 } 386 }
387 387
388
388 void 389 void
389 imap4d_child_signal_setup (RETSIGTYPE (*handler) (int signo)) 390 imap4d_child_signal_setup (RETSIGTYPE (*handler) (int signo))
390 { 391 {
391 static int sigtab[] = { SIGILL, SIGBUS, SIGFPE, SIGSEGV, SIGSTOP, SIGPIPE, 392 static int sigtab[] = { SIGPIPE, SIGABRT, SIGINT, SIGQUIT,
392 SIGABRT, SIGINT, SIGQUIT, SIGTERM, SIGHUP, SIGALRM }; 393 SIGTERM, SIGHUP, SIGALRM };
393 mu_set_signals (handler, sigtab, MU_ARRAY_SIZE (sigtab)); 394 mu_set_signals (handler, sigtab, MU_ARRAY_SIZE (sigtab));
394 } 395 }
395 396
...@@ -399,8 +400,40 @@ imap4d_mainloop (int ifd, int ofd) ...@@ -399,8 +400,40 @@ imap4d_mainloop (int ifd, int ofd)
399 imap4d_tokbuf_t tokp; 400 imap4d_tokbuf_t tokp;
400 char *text; 401 char *text;
401 int debug_mode = isatty (ifd); 402 int debug_mode = isatty (ifd);
403 int signo;
402 404
405 if ((signo = setjmp (child_jmp)))
406 {
407 mu_diag_output (MU_DIAG_CRIT, _("got signal `%s'"), strsignal (signo));
408 switch (signo)
409 {
410 case SIGTERM:
411 case SIGHUP:
412 signo = ERR_TERMINATE;
413 break;
414
415 case SIGALRM:
416 signo = ERR_TIMEOUT;
417 break;
418
419 case SIGPIPE:
420 signo = ERR_NO_OFILE;
421 break;
422
423 default:
424 signo = ERR_SIGNAL;
425 }
426 imap4d_bye (signo);
427 }
428 else
429 {
430 /* Restore default handling for these signals: */
431 static int defsigtab[] = { SIGILL, SIGBUS, SIGFPE, SIGSEGV, SIGSTOP };
432 mu_set_signals (SIG_DFL, defsigtab, MU_ARRAY_SIZE (defsigtab));
433 /* Set child-specific signal handlers */
403 imap4d_child_signal_setup (imap4d_child_signal); 434 imap4d_child_signal_setup (imap4d_child_signal);
435 }
436
404 io_setio (ifd, ofd); 437 io_setio (ifd, ofd);
405 438
406 if (imap4d_preauth_setup (ifd) == 0) 439 if (imap4d_preauth_setup (ifd) == 0)
...@@ -484,6 +517,17 @@ imap4d_check_home_dir (const char *dir, uid_t uid, gid_t gid) ...@@ -484,6 +517,17 @@ imap4d_check_home_dir (const char *dir, uid_t uid, gid_t gid)
484 return 0; 517 return 0;
485 } 518 }
486 519
520
521 jmp_buf master_jmp;
522
523 RETSIGTYPE
524 imap4d_master_signal (int signo)
525 {
526 longjmp (master_jmp, signo);
527 }
528
529
530
487 int 531 int
488 main (int argc, char **argv) 532 main (int argc, char **argv)
489 { 533 {
...@@ -578,6 +622,27 @@ main (int argc, char **argv) ...@@ -578,6 +622,27 @@ main (int argc, char **argv)
578 } 622 }
579 623
580 /* Set the signal handlers. */ 624 /* Set the signal handlers. */
625 if ((status = setjmp (master_jmp)))
626 {
627 int code;
628 mu_diag_output (MU_DIAG_CRIT, _("MASTER: exiting on signal (%s)"),
629 strsignal (status));
630 switch (status)
631 {
632 case SIGTERM:
633 case SIGHUP:
634 case SIGQUIT:
635 case SIGINT:
636 code = EX_OK;
637 break;
638
639 default:
640 code = EX_SOFTWARE;
641 break;
642 }
643
644 exit (code);
645 }
581 mu_set_signals (imap4d_master_signal, sigtab, MU_ARRAY_SIZE (sigtab)); 646 mu_set_signals (imap4d_master_signal, sigtab, MU_ARRAY_SIZE (sigtab));
582 647
583 mu_stdstream_strerr_setup (mu_log_syslog ? 648 mu_stdstream_strerr_setup (mu_log_syslog ?
......
...@@ -202,6 +202,7 @@ extern int imap4d_transcript; ...@@ -202,6 +202,7 @@ extern int imap4d_transcript;
202 extern mu_list_t imap4d_id_list; 202 extern mu_list_t imap4d_id_list;
203 extern int imap4d_argc; 203 extern int imap4d_argc;
204 extern char **imap4d_argv; 204 extern char **imap4d_argv;
205 extern jmp_buf child_jmp;
205 206
206 /* Input functions */ 207 /* Input functions */
207 extern mu_stream_t iostream; 208 extern mu_stream_t iostream;
...@@ -334,6 +335,9 @@ extern RETSIGTYPE imap4d_master_signal (int); ...@@ -334,6 +335,9 @@ extern RETSIGTYPE imap4d_master_signal (int);
334 extern RETSIGTYPE imap4d_child_signal (int); 335 extern RETSIGTYPE imap4d_child_signal (int);
335 extern int imap4d_bye (int); 336 extern int imap4d_bye (int);
336 extern int imap4d_bye0 (int reason, struct imap4d_command *command); 337 extern int imap4d_bye0 (int reason, struct imap4d_command *command);
338 void imap4d_enter_critical (void);
339 void imap4d_leave_critical (void);
340
337 341
338 /* Namespace functions */ 342 /* Namespace functions */
339 extern mu_list_t namespace[NS_MAX]; 343 extern mu_list_t namespace[NS_MAX];
......
...@@ -182,12 +182,17 @@ imap4d_rename (struct imap4d_command *command, imap4d_tokbuf_t tok) ...@@ -182,12 +182,17 @@ imap4d_rename (struct imap4d_command *command, imap4d_tokbuf_t tok)
182 if (mu_mailbox_get_message (inbox, no, &message) == 0) 182 if (mu_mailbox_get_message (inbox, no, &message) == 0)
183 { 183 {
184 mu_attribute_t attr = NULL; 184 mu_attribute_t attr = NULL;
185
186 imap4d_enter_critical ();
185 mu_mailbox_append_message (newmbox, message); 187 mu_mailbox_append_message (newmbox, message);
188 imap4d_leave_critical ();
186 mu_message_get_attribute (message, &attr); 189 mu_message_get_attribute (message, &attr);
187 mu_attribute_set_deleted (attr); 190 mu_attribute_set_deleted (attr);
188 } 191 }
189 } 192 }
193 imap4d_enter_critical ();
190 mu_mailbox_expunge (inbox); 194 mu_mailbox_expunge (inbox);
195 imap4d_leave_critical ();
191 mu_mailbox_close (inbox); 196 mu_mailbox_close (inbox);
192 mu_mailbox_destroy (&inbox); 197 mu_mailbox_destroy (&inbox);
193 } 198 }
......
...@@ -46,8 +46,10 @@ imap4d_select0 (struct imap4d_command *command, const char *mboxname, ...@@ -46,8 +46,10 @@ imap4d_select0 (struct imap4d_command *command, const char *mboxname,
46 currently selected mailbox without doing an expunge. */ 46 currently selected mailbox without doing an expunge. */
47 if (mbox) 47 if (mbox)
48 { 48 {
49 imap4d_enter_critical ();
49 mu_mailbox_sync (mbox); 50 mu_mailbox_sync (mbox);
50 mu_mailbox_close (mbox); 51 mu_mailbox_close (mbox);
52 imap4d_leave_critical ();
51 mu_mailbox_destroy (&mbox); 53 mu_mailbox_destroy (&mbox);
52 /* Destroy the old uid table. */ 54 /* Destroy the old uid table. */
53 imap4d_sync (); 55 imap4d_sync ();
......
...@@ -15,57 +15,34 @@ ...@@ -15,57 +15,34 @@
15 You should have received a copy of the GNU General Public License 15 You should have received a copy of the GNU General Public License
16 along with GNU Mailutils. If not, see <http://www.gnu.org/licenses/>. */ 16 along with GNU Mailutils. If not, see <http://www.gnu.org/licenses/>. */
17 17
18 #define __USE_MISC
19 #include "imap4d.h" 18 #include "imap4d.h"
20 19
21 /* Default signal handler to call the imap4d_bye() function */ 20 jmp_buf child_jmp;
21 static int __critical_section;
22 static int __got_signal;
22 23
23 RETSIGTYPE 24 void
24 imap4d_master_signal (int signo) 25 imap4d_enter_critical ()
25 { 26 {
26 int code; 27 __critical_section = 1;
27 28 if (__got_signal)
28 mu_diag_output (MU_DIAG_CRIT, _("MASTER: exiting on signal (%s)"), 29 __critical_section++;
29 strsignal (signo)); 30 }
30 switch (signo)
31 {
32 case SIGTERM:
33 case SIGHUP:
34 case SIGQUIT:
35 case SIGINT:
36 code = EX_OK;
37 break;
38
39 default:
40 code = EX_SOFTWARE;
41 break;
42 }
43 31
44 exit (code); 32 void
33 imap4d_leave_critical ()
34 {
35 if (__got_signal && __critical_section != 2)
36 longjmp (child_jmp, __got_signal);
37 __critical_section = 0;
45 } 38 }
46 39
47 RETSIGTYPE 40 RETSIGTYPE
48 imap4d_child_signal (int signo) 41 imap4d_child_signal (int signo)
49 { 42 {
50 imap4d_child_signal_setup (SIG_IGN); 43 imap4d_child_signal_setup (SIG_IGN);
51 mu_diag_output (MU_DIAG_CRIT, _("got signal `%s'"), strsignal (signo)); 44 if (__critical_section)
52 switch (signo) 45 __got_signal = signo;
53 { 46 else
54 case SIGTERM: 47 longjmp (child_jmp, signo);
55 case SIGHUP:
56 signo = ERR_TERMINATE;
57 break;
58
59 case SIGALRM:
60 signo = ERR_TIMEOUT;
61 break;
62
63 case SIGPIPE:
64 signo = ERR_NO_OFILE;
65 break;
66
67 default:
68 signo = ERR_SIGNAL;
69 }
70 imap4d_bye (signo);
71 } 48 }
......
...@@ -88,7 +88,9 @@ imap4d_status (struct imap4d_command *command, imap4d_tokbuf_t tok) ...@@ -88,7 +88,9 @@ imap4d_status (struct imap4d_command *command, imap4d_tokbuf_t tok)
88 88
89 /* We may be opening the current mailbox, so make sure the attributes are 89 /* We may be opening the current mailbox, so make sure the attributes are
90 preserved */ 90 preserved */
91 imap4d_enter_critical ();
91 mu_mailbox_sync (mbox); 92 mu_mailbox_sync (mbox);
93 imap4d_leave_critical ();
92 94
93 status = mu_mailbox_create_default (&smbox, mailbox_name); 95 status = mu_mailbox_create_default (&smbox, mailbox_name);
94 if (status == 0) 96 if (status == 0)
......
...@@ -56,5 +56,6 @@ sievemod_DATA=\ ...@@ -56,5 +56,6 @@ sievemod_DATA=\
56 EXTRA_DIST=\ 56 EXTRA_DIST=\
57 $(sievemod_DATA)\ 57 $(sievemod_DATA)\
58 sieve-core.scm\ 58 sieve-core.scm\
59 sieve2scm.scmi 59 sieve2scm.scmi\
60 guimb.scmi
60 61
......