diff options
author | smitsohu <smitsohu@gmail.com> | 2019-01-19 23:50:47 +0100 |
---|---|---|
committer | smitsohu <smitsohu@gmail.com> | 2019-01-19 23:50:47 +0100 |
commit | 40be34049db1a2f39419b1fa2a371d68ad75439f (patch) | |
tree | 26241adbb96471e4abada77984f13fb9c5f918e9 | |
parent | improve chroot error handling (diff) | |
download | firejail-40be34049db1a2f39419b1fa2a371d68ad75439f.tar.gz firejail-40be34049db1a2f39419b1fa2a371d68ad75439f.tar.zst firejail-40be34049db1a2f39419b1fa2a371d68ad75439f.zip |
signal handler fixes/improvements
-rw-r--r-- | src/firejail/firejail.h | 1 | ||||
-rw-r--r-- | src/firejail/join.c | 21 | ||||
-rw-r--r-- | src/firejail/main.c | 48 | ||||
-rw-r--r-- | src/firejail/sandbox.c | 51 | ||||
-rw-r--r-- | src/firejail/util.c | 31 |
5 files changed, 131 insertions, 21 deletions
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index a3d3dfcd4..03ad25f75 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h | |||
@@ -551,6 +551,7 @@ void disable_file_or_dir(const char *fname); | |||
551 | void disable_file_path(const char *path, const char *file); | 551 | void disable_file_path(const char *path, const char *file); |
552 | int safe_fd(const char *path, int flags); | 552 | int safe_fd(const char *path, int flags); |
553 | int invalid_sandbox(const pid_t pid); | 553 | int invalid_sandbox(const pid_t pid); |
554 | int has_handler(pid_t pid, int signal); | ||
554 | void enter_network_namespace(pid_t pid); | 555 | void enter_network_namespace(pid_t pid); |
555 | 556 | ||
556 | // Get info regarding the last kernel mount operation from /proc/self/mountinfo | 557 | // Get info regarding the last kernel mount operation from /proc/self/mountinfo |
diff --git a/src/firejail/join.c b/src/firejail/join.c index 4259644f7..d05a4a465 100644 --- a/src/firejail/join.c +++ b/src/firejail/join.c | |||
@@ -41,7 +41,15 @@ static void signal_handler(int sig){ | |||
41 | exit(sig); | 41 | exit(sig); |
42 | } | 42 | } |
43 | 43 | ||
44 | 44 | static void install_handler(void) { | |
45 | struct sigaction sga; | ||
46 | |||
47 | // handle SIGTERM | ||
48 | sigemptyset(&sga.sa_mask); | ||
49 | sga.sa_handler = signal_handler; | ||
50 | sga.sa_flags = 0; | ||
51 | sigaction(SIGTERM, &sga, NULL); | ||
52 | } | ||
45 | 53 | ||
46 | static void extract_x11_display(pid_t pid) { | 54 | static void extract_x11_display(pid_t pid) { |
47 | char *fname; | 55 | char *fname; |
@@ -465,8 +473,19 @@ void join(pid_t pid, int argc, char **argv, int index) { | |||
465 | } | 473 | } |
466 | 474 | ||
467 | int status = 0; | 475 | int status = 0; |
476 | //***************************** | ||
477 | // following code is signal-safe | ||
478 | |||
479 | install_handler(); | ||
480 | |||
468 | // wait for the child to finish | 481 | // wait for the child to finish |
469 | waitpid(child, &status, 0); | 482 | waitpid(child, &status, 0); |
483 | |||
484 | // restore default signal action | ||
485 | signal(SIGTERM, SIG_DFL); | ||
486 | |||
487 | // end of signal-safe code | ||
488 | //***************************** | ||
470 | flush_stdin(); | 489 | flush_stdin(); |
471 | 490 | ||
472 | if (WIFEXITED(status)) { | 491 | if (WIFEXITED(status)) { |
diff --git a/src/firejail/main.c b/src/firejail/main.c index faa4972e2..61f507f36 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c | |||
@@ -157,12 +157,35 @@ static void myexit(int rv) { | |||
157 | exit(rv); | 157 | exit(rv); |
158 | } | 158 | } |
159 | 159 | ||
160 | static void my_handler(int s){ | 160 | static void my_handler(int s) { |
161 | EUID_ROOT(); | ||
162 | fmessage("\nParent received signal %d, shutting down the child process...\n", s); | 161 | fmessage("\nParent received signal %d, shutting down the child process...\n", s); |
163 | logsignal(s); | 162 | logsignal(s); |
164 | kill(child, SIGTERM); | 163 | if (waitpid(child, NULL, WNOHANG) == 0) { |
165 | myexit(1); | 164 | if (has_handler(child, s)) // signals are not delivered if there is no handler yet |
165 | kill(child, s); | ||
166 | else | ||
167 | kill(child, SIGKILL); | ||
168 | waitpid(child, NULL, 0); | ||
169 | } | ||
170 | myexit(s); | ||
171 | } | ||
172 | |||
173 | static void install_handler(void) { | ||
174 | struct sigaction sga; | ||
175 | |||
176 | // block SIGTERM while handling SIGINT | ||
177 | sigemptyset(&sga.sa_mask); | ||
178 | sigaddset(&sga.sa_mask, SIGTERM); | ||
179 | sga.sa_handler = my_handler; | ||
180 | sga.sa_flags = 0; | ||
181 | sigaction(SIGINT, &sga, NULL); | ||
182 | |||
183 | // block SIGINT while handling SIGTERM | ||
184 | sigemptyset(&sga.sa_mask); | ||
185 | sigaddset(&sga.sa_mask, SIGINT); | ||
186 | sga.sa_handler = my_handler; | ||
187 | sga.sa_flags = 0; | ||
188 | sigaction(SIGTERM, &sga, NULL); | ||
166 | } | 189 | } |
167 | 190 | ||
168 | // return 1 if error, 0 if a valid pid was found | 191 | // return 1 if error, 0 if a valid pid was found |
@@ -2601,16 +2624,25 @@ int main(int argc, char **argv) { | |||
2601 | flock(lockfd_network, LOCK_UN); | 2624 | flock(lockfd_network, LOCK_UN); |
2602 | close(lockfd_network); | 2625 | close(lockfd_network); |
2603 | } | 2626 | } |
2627 | EUID_USER(); | ||
2628 | |||
2629 | int status = 0; | ||
2630 | //***************************** | ||
2631 | // following code is signal-safe | ||
2604 | 2632 | ||
2605 | // handle CTRL-C in parent | 2633 | // handle CTRL-C in parent |
2606 | signal (SIGINT, my_handler); | 2634 | install_handler(); |
2607 | signal (SIGTERM, my_handler); | ||
2608 | 2635 | ||
2609 | // wait for the child to finish | 2636 | // wait for the child to finish |
2610 | EUID_USER(); | ||
2611 | int status = 0; | ||
2612 | waitpid(child, &status, 0); | 2637 | waitpid(child, &status, 0); |
2613 | 2638 | ||
2639 | // restore default signal actions | ||
2640 | signal(SIGTERM, SIG_DFL); | ||
2641 | signal(SIGINT, SIG_DFL); | ||
2642 | |||
2643 | // end of signal-safe code | ||
2644 | //***************************** | ||
2645 | |||
2614 | // free globals | 2646 | // free globals |
2615 | if (cfg.profile) { | 2647 | if (cfg.profile) { |
2616 | ProfileEntry *prf = cfg.profile; | 2648 | ProfileEntry *prf = cfg.profile; |
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c index 475779f47..ba9a36250 100644 --- a/src/firejail/sandbox.c +++ b/src/firejail/sandbox.c | |||
@@ -53,6 +53,7 @@ static int force_nonewprivs = 0; | |||
53 | 53 | ||
54 | static int monitored_pid = 0; | 54 | static int monitored_pid = 0; |
55 | static void sandbox_handler(int sig){ | 55 | static void sandbox_handler(int sig){ |
56 | usleep(10000); // don't race to print a message | ||
56 | fmessage("\nChild received signal %d, shutting down the sandbox...\n", sig); | 57 | fmessage("\nChild received signal %d, shutting down the sandbox...\n", sig); |
57 | 58 | ||
58 | // broadcast sigterm to all processes in the group | 59 | // broadcast sigterm to all processes in the group |
@@ -81,10 +82,8 @@ static void sandbox_handler(int sig){ | |||
81 | monsec--; | 82 | monsec--; |
82 | } | 83 | } |
83 | free(monfile); | 84 | free(monfile); |
84 | |||
85 | } | 85 | } |
86 | 86 | ||
87 | |||
88 | // broadcast a SIGKILL | 87 | // broadcast a SIGKILL |
89 | kill(-1, SIGKILL); | 88 | kill(-1, SIGKILL); |
90 | flush_stdin(); | 89 | flush_stdin(); |
@@ -92,6 +91,23 @@ static void sandbox_handler(int sig){ | |||
92 | exit(sig); | 91 | exit(sig); |
93 | } | 92 | } |
94 | 93 | ||
94 | static void install_handler(void) { | ||
95 | struct sigaction sga; | ||
96 | |||
97 | // block SIGTERM while handling SIGINT | ||
98 | sigemptyset(&sga.sa_mask); | ||
99 | sigaddset(&sga.sa_mask, SIGTERM); | ||
100 | sga.sa_handler = sandbox_handler; | ||
101 | sga.sa_flags = 0; | ||
102 | sigaction(SIGINT, &sga, NULL); | ||
103 | |||
104 | // block SIGINT while handling SIGTERM | ||
105 | sigemptyset(&sga.sa_mask); | ||
106 | sigaddset(&sga.sa_mask, SIGINT); | ||
107 | sga.sa_handler = sandbox_handler; | ||
108 | sga.sa_flags = 0; | ||
109 | sigaction(SIGTERM, &sga, NULL); | ||
110 | } | ||
95 | 111 | ||
96 | static void set_caps(void) { | 112 | static void set_caps(void) { |
97 | if (arg_caps_drop_all) | 113 | if (arg_caps_drop_all) |
@@ -123,7 +139,6 @@ static void save_nogroups(void) { | |||
123 | fprintf(stderr, "Error: cannot save nogroups state\n"); | 139 | fprintf(stderr, "Error: cannot save nogroups state\n"); |
124 | exit(1); | 140 | exit(1); |
125 | } | 141 | } |
126 | |||
127 | } | 142 | } |
128 | 143 | ||
129 | static void save_nonewprivs(void) { | 144 | static void save_nonewprivs(void) { |
@@ -235,9 +250,17 @@ static void chk_chroot(void) { | |||
235 | } | 250 | } |
236 | 251 | ||
237 | static int monitor_application(pid_t app_pid) { | 252 | static int monitor_application(pid_t app_pid) { |
253 | EUID_ASSERT(); | ||
238 | monitored_pid = app_pid; | 254 | monitored_pid = app_pid; |
239 | signal (SIGTERM, sandbox_handler); | 255 | |
240 | EUID_USER(); | 256 | // block signals and install handler |
257 | sigset_t oldmask, newmask; | ||
258 | sigemptyset(&oldmask); | ||
259 | sigemptyset(&newmask); | ||
260 | sigaddset(&newmask, SIGTERM); | ||
261 | sigaddset(&newmask, SIGINT); | ||
262 | sigprocmask(SIG_BLOCK, &newmask, &oldmask); | ||
263 | install_handler(); | ||
241 | 264 | ||
242 | // handle --timeout | 265 | // handle --timeout |
243 | int options = 0;; | 266 | int options = 0;; |
@@ -260,16 +283,25 @@ static int monitor_application(pid_t app_pid) { | |||
260 | 283 | ||
261 | pid_t rv; | 284 | pid_t rv; |
262 | do { | 285 | do { |
286 | // handle signals asynchronously | ||
287 | sigprocmask(SIG_SETMASK, &oldmask, NULL); | ||
288 | |||
263 | rv = waitpid(-1, &status, options); | 289 | rv = waitpid(-1, &status, options); |
264 | if (rv == -1) | 290 | |
291 | // block signals again | ||
292 | sigprocmask(SIG_BLOCK, &newmask, NULL); | ||
293 | |||
294 | if (rv == -1) { // we can get here if we have processes joining the sandbox (ECHILD) | ||
295 | sleep(1); | ||
265 | break; | 296 | break; |
297 | } | ||
266 | 298 | ||
267 | // handle --timeout | 299 | // handle --timeout |
268 | if (options) { | 300 | if (options) { |
269 | if (--timeout == 0) { | 301 | if (--timeout == 0) { |
270 | kill(-1, SIGTERM); | 302 | kill(-1, SIGTERM); |
271 | flush_stdin(); | ||
272 | sleep(1); | 303 | sleep(1); |
304 | flush_stdin(); | ||
273 | _exit(1); | 305 | _exit(1); |
274 | } | 306 | } |
275 | else | 307 | else |
@@ -279,11 +311,6 @@ static int monitor_application(pid_t app_pid) { | |||
279 | while(rv != monitored_pid); | 311 | while(rv != monitored_pid); |
280 | if (arg_debug) | 312 | if (arg_debug) |
281 | printf("Sandbox monitor: waitpid %d retval %d status %d\n", monitored_pid, rv, status); | 313 | printf("Sandbox monitor: waitpid %d retval %d status %d\n", monitored_pid, rv, status); |
282 | if (rv == -1) { // we can get here if we have processes joining the sandbox (ECHILD) | ||
283 | if (arg_debug) | ||
284 | perror("waitpid"); | ||
285 | sleep(1); | ||
286 | } | ||
287 | 314 | ||
288 | DIR *dir; | 315 | DIR *dir; |
289 | if (!(dir = opendir("/proc"))) { | 316 | if (!(dir = opendir("/proc"))) { |
diff --git a/src/firejail/util.c b/src/firejail/util.c index 8c474f966..6b30ab5fe 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c | |||
@@ -1264,6 +1264,37 @@ int invalid_sandbox(const pid_t pid) { | |||
1264 | return 0; | 1264 | return 0; |
1265 | } | 1265 | } |
1266 | 1266 | ||
1267 | int has_handler(pid_t pid, int signal) { | ||
1268 | if (signal > 0) { | ||
1269 | char *fname; | ||
1270 | if (asprintf(&fname, "/proc/%d/status", pid) == -1) | ||
1271 | errExit("asprintf"); | ||
1272 | EUID_ROOT(); | ||
1273 | FILE *fp = fopen(fname, "re"); | ||
1274 | EUID_USER(); | ||
1275 | free(fname); | ||
1276 | if (fp) { | ||
1277 | char buf[BUFLEN]; | ||
1278 | while (fgets(buf, BUFLEN, fp)) { | ||
1279 | if (strncmp(buf, "SigCgt:", 7) == 0) { | ||
1280 | char *ptr = buf + 7; | ||
1281 | unsigned long long val; | ||
1282 | if (sscanf(ptr, "%llx", &val) != 1) { | ||
1283 | fprintf(stderr, "Error: cannot read /proc file\n"); | ||
1284 | exit(1); | ||
1285 | } | ||
1286 | val >>= (signal - 1); | ||
1287 | val &= 1; | ||
1288 | fclose(fp); | ||
1289 | return val; // 1 if process has a handler for the signal, else 0 | ||
1290 | } | ||
1291 | } | ||
1292 | fclose(fp); | ||
1293 | } | ||
1294 | } | ||
1295 | return 0; | ||
1296 | } | ||
1297 | |||
1267 | void enter_network_namespace(pid_t pid) { | 1298 | void enter_network_namespace(pid_t pid) { |
1268 | // in case the pid is that of a firejail process, use the pid of the first child process | 1299 | // in case the pid is that of a firejail process, use the pid of the first child process |
1269 | pid_t child = switch_to_child(pid); | 1300 | pid_t child = switch_to_child(pid); |