github-starred/firejail
2
0
Fork 0
mirror of https://github.com/netblue30/firejail.git synced 2026-05-15 14:16:14 -06:00
Code Issues 531 Projects Packages Wiki Activity Actions 6

bugfix: fix potential deadlock with flock + SIGTSTP (#6750)

Browse source 
As reported by @hlein in #6729, if a firejailed process is `^Z`'d at the
wrong time during startup, other firejail processes will be unable to
proceed because they'll wait forever for an flock on
`/run/firejail/firejail-run.lock`.

This fixes that by installing a signal handler to catch SIGTSTP (Ctrl-Z)
before acquiring locks. The handler increments a flag to allow for
re-sending the SIGTSTP signal after locks have been released.

Deadlock Reproducer:

Note: The length of the sleep should be adjusted until the debug output
resembles the output below:

    $ firejail --debug id & P=$!;sleep 0.0005;kill -TSTP $P
    [1] 16130
    Looking for kernel processes
    Found kthreadd process, we are not running in a sandbox
    pid=16130: locking /run/firejail/firejail-run.lock ...
    pid=16130: locked /run/firejail/firejail-run.lock

    [1]+  Stopped                 firejail --debug id

Further calls to firejail will hang due to the stopped process holding
the firejail-run lock.

    $ firejail id
    ^C

With this commit:

    $ firejail --debug id & P=$!;sleep 0.0005;kill -TSTP $P
    [1] 16504
    Looking for kernel processes
    Found kthreadd process, we are not running in a sandbox
    pid=16504: locking /run/firejail/firejail-run.lock ...
    pid=16504: locked /run/firejail/firejail-run.lock
    pid=16504: caught SIGTSTP while locks are held
    pid=16504: unlocking /run/firejail/firejail-run.lock ...
    pid=16504: unlocked /run/firejail/firejail-run.lock
    pid=16504: resending caught SIGTSTP

    [1]+  Stopped                 firejail --debug id

Due to the locks being properly released before the process is stopped,
new firejail processes will not hang while acquiring the lock.

Fixes #6729.

Reported-by: @hlein
This commit is contained in:
jlimor-kl 2025-05-21 06:18:55 -04:00 • committed by GitHub
parent b84e2591a8
commit f4b8c6dbb9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 49 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

49
src/firejail/preproc.c
View file

@ -24,8 +24,53 @@
#include <sys/types.h>
#include <dirent.h>
#include <fcntl.h>
#include <signal.h>
static int tmpfs_mounted = 0;
static volatile sig_atomic_t caught_tstp = 0;
static struct sigaction backup_tstp_directory_action;
static struct sigaction backup_tstp_network_action;
// We need to ignore SIGTSTP while we hold flock(s), otherwise that signal
// could stop this process before the locks are released, causing future
// firejail processes to be stuck during startup (see #6729).
static void handle_sigtstp(int signo) {
(void) signo;
if (arg_debug) {
long pid = (long)getpid();
printf("pid=%ld: caught SIGTSTP while holding a lock\n", pid);
}
caught_tstp++;
}
static void install_ignore_tstp_signal_handler(struct sigaction* backup_action) {
struct sigaction sa_ignore;
sa_ignore.sa_handler = handle_sigtstp;
sa_ignore.sa_flags = 0;
sigemptyset(&sa_ignore.sa_mask);
if (sigaction(SIGTSTP, &sa_ignore, backup_action) == -1)
errExit("sigaction");
}
static void uninstall_ignore_tstp_signal_handler(struct sigaction* backup_action) {
if (sigaction(SIGTSTP, backup_action, NULL) == -1)
errExit("sigaction");
if (caught_tstp > 0) {
if (arg_debug) {
long pid = (long)getpid();
printf("pid=%ld: resending caught SIGTSTP\n", pid);
}
// We can do this even in the case of nested locks, as in that
// case caught_tstp will just be incremented and eventually the
// outermost unlock will restore the original handler before
// resending SIGTSTP.
caught_tstp = 0;
raise(SIGTSTP);
}
}
static void preproc_lock_file(const char *path, int *lockfd_ptr) {
assert(path);
@ -93,19 +138,23 @@ static void preproc_unlock_file(const char *path, int *lockfd_ptr) {
}
void preproc_lock_firejail_dir(void) {
install_ignore_tstp_signal_handler(&backup_tstp_directory_action);
preproc_lock_file(RUN_DIRECTORY_LOCK_FILE, &lockfd_directory);
}
void preproc_unlock_firejail_dir(void) {
preproc_unlock_file(RUN_DIRECTORY_LOCK_FILE, &lockfd_directory);
uninstall_ignore_tstp_signal_handler(&backup_tstp_directory_action);
}
void preproc_lock_firejail_network_dir(void) {
install_ignore_tstp_signal_handler(&backup_tstp_network_action);
preproc_lock_file(RUN_NETWORK_LOCK_FILE, &lockfd_network);
}
void preproc_unlock_firejail_network_dir(void) {
preproc_unlock_file(RUN_NETWORK_LOCK_FILE, &lockfd_network);
uninstall_ignore_tstp_signal_handler(&backup_tstp_network_action);
}
// build /run/firejail directory