[GH-ISSUE #6729] Deadlock on /run/firejail/firejail-run.lock when a firejailed process is suspended

gitea-mirror commented

2026-05-05 09:55:52 -06:00

Owner

Originally created by @hlein on GitHub (May 1, 2025).
Original GitHub issue: https://github.com/netblue30/firejail/issues/6729

Description

If a firejail'ed process is ^Z'd at the wrong time during startup, other firejail processes will be unable to proceed because they'll wait forever for a flock on /run/firejail/firejail-run.lock

Steps to Reproduce

Run something firejail'ed that will take a while to start. In my case I was running a make target that ssh'ed to a remote host via a SOCKS proxy listener that was dead.
control-z the parent (the make in my case)
in another terminal try to run any firejail'ed thing
never see startup messages...

Note, this is racy (interrupting between lock and unlock) so I don't expect it to be reproducible every time.

Expected behavior

Separate/independent firejails should proceed, or if not, perhaps output an error (upon timeout?).

Actual behavior

Any new firejail process hangs indefinitely at startup.

Behavior without a profile

I think as long as we grab a lock on /run/firejail/firejail-run.lock, and allow SIGSTP to interrupt while we hold the lock, we are susceptible.

Additional context

Maybe we can block SIGSTP in the time between preproc_lock_firejail_dir() and preproc_unlock_firejail_dir()? And/or use LOCK_NB and busy-wait a reasonable amount of time so we can abort.

Environment

Name/version/arch of the Linux kernel (uname -srm): Linux 6.6.83-gentoo x86_64
Name/version of the Linux distribution (e.g. "Ubuntu 20.04" or "Arch Linux"): Gentoo Linux
Name/version of the relevant program(s)/package(s) (e.g. "firefox 134.0-1,
mesa 1:24.3.3-2"): openssh-10.0_p1 (but I think it's not dependent on that)
Version of Firejail (firejail --version): 0.9.74
If you use a development version of firejail, also the commit from which it
was compiled (git rev-parse HEAD):

Checklist

The issues is caused by firejail (i.e. running the program by path (e.g. /usr/bin/vlc) "fixes" it).
I can reproduce the issue without custom modifications (e.g. globals.local).
The program has a profile. (If not, request one in https://github.com/netblue30/firejail/issues/1139)
The profile (and redirect profile if exists) hasn't already been fixed upstream.
I have performed a short search for similar issues (to avoid opening a duplicate).
- I'm aware of browser-allow-drm yes/browser-disable-u2f no in firejail.config to allow DRM/U2F in browsers.
[n/a] I used --profile=PROFILENAME to set the right profile. (Only relevant for AppImages)

Originally created by @hlein on GitHub (May 1, 2025). Original GitHub issue: https://github.com/netblue30/firejail/issues/6729 ### Description If a firejail'ed process is `^Z`'d at the wrong time during startup, other firejail processes will be unable to proceed because they'll wait forever for a flock on `/run/firejail/firejail-run.lock` ### Steps to Reproduce 1. Run something firejail'ed that will take a while to start. In my case I was running a `make` target that `ssh`'ed to a remote host via a SOCKS proxy listener that was dead. 2. control-z the parent (the `make` in my case) 3. in another terminal try to run any firejail'ed thing 4. never see startup messages... Note, this is racy (interrupting between lock and unlock) so I don't expect it to be reproducible every time. ### Expected behavior Separate/independent firejails should proceed, or if not, perhaps output an error (upon timeout?). ### Actual behavior Any new firejail process hangs indefinitely at startup. ### Behavior without a profile I think as long as we grab a lock on `/run/firejail/firejail-run.lock`, and allow `SIGSTP` to interrupt while we hold the lock, we are susceptible. ### Additional context Maybe we can block `SIGSTP` in the time between `preproc_lock_firejail_dir()` and `preproc_unlock_firejail_dir()`? And/or use `LOCK_NB` and busy-wait a reasonable amount of time so we can abort. ### Environment - Name/version/arch of the Linux kernel (`uname -srm`): Linux 6.6.83-gentoo x86_64 - Name/version of the Linux distribution (e.g. "Ubuntu 20.04" or "Arch Linux"): Gentoo Linux - Name/version of the relevant program(s)/package(s) (e.g. "firefox 134.0-1, mesa 1:24.3.3-2"): openssh-10.0_p1 (but I think it's not dependent on that) - Version of Firejail (`firejail --version`): 0.9.74 - If you use a development version of firejail, also the commit from which it was compiled (`git rev-parse HEAD`): ### Checklist  - [x] The issues is caused by firejail (i.e. running the program by path (e.g. `/usr/bin/vlc`) "fixes" it). - [x] I can reproduce the issue without custom modifications (e.g. globals.local). - [x] The program has a profile. (If not, request one in `https://github.com/netblue30/firejail/issues/1139`) - [x] The profile (and redirect profile if exists) hasn't already been fixed [upstream](https://github.com/netblue30/firejail/tree/master/etc). - [x] I have performed a short search for similar issues (to avoid opening a duplicate). - [x] I'm aware of `browser-allow-drm yes`/`browser-disable-u2f no` in `firejail.config` to allow DRM/U2F in browsers. - [n/a] I used `--profile=PROFILENAME` to set the right profile. (Only relevant for AppImages)

gitea-mirror

2026-05-05 09:55:52 -06:00

closed this issue
added the
bug
label

gitea-mirror commented

2026-05-05 09:55:55 -06:00

Author

Owner

@kmk3 commented on GitHub (May 2, 2025):

I think as long as we grab a lock on /run/firejail/firejail-run.lock, and
allow SIGSTP to interrupt while we hold the lock, we are susceptible.

Maybe we can block SIGSTP in the time between preproc_lock_firejail_dir()
and preproc_unlock_firejail_dir()? And/or use LOCK_NB and busy-wait a
reasonable amount of time so we can abort.

Makes sense to me.

@spiiroin Any thoughts on this?

Relates to:

#6307

@kmk3 commented on GitHub (May 2, 2025): > I think as long as we grab a lock on `/run/firejail/firejail-run.lock`, and > allow `SIGSTP` to interrupt while we hold the lock, we are susceptible. > Maybe we can block `SIGSTP` in the time between `preproc_lock_firejail_dir()` > and `preproc_unlock_firejail_dir()`? And/or use `LOCK_NB` and busy-wait a > reasonable amount of time so we can abort. Makes sense to me. @spiiroin Any thoughts on this? Relates to: * #6307

gitea-mirror commented

2026-05-05 09:55:56 -06:00

Author

Owner

@spiiroin commented on GitHub (May 5, 2025):

@kmk3 I guess the worst part is that should somebody be bitten by this deadlock, it probably is not that easy to figure out what is going on. As the lock is supposed to be held only very briefly, I see no harm with blocking stop for that duration. And non-blocking lock attempts in a loop would allow sensible diagnostic output & bailout in case of failure like this (or something similar). At this stage the program is still starting up, so there is not too much to lose yet etc

@spiiroin commented on GitHub (May 5, 2025): @kmk3 I guess the worst part is that should somebody be bitten by this deadlock, it probably is not that easy to figure out what is going on. As the lock is supposed to be held only very briefly, I see no harm with blocking stop for that duration. And non-blocking lock attempts in a loop would allow sensible diagnostic output & bailout in case of failure like this (or something similar). At this stage the program is still starting up, so there is not too much to lose yet etc

gitea-mirror commented

2026-05-05 09:55:57 -06:00

Author

Owner

@kmk3 commented on GitHub (May 7, 2025):

@spiiroin

I guess the worst part is that should somebody be bitten by this deadlock, it
probably is not that easy to figure out what is going on. As the lock is
supposed to be held only very briefly, I see no harm with blocking stop for
that duration. And non-blocking lock attempts in a loop would allow sensible
diagnostic output & bailout in case of failure like this (or something
similar). At this stage the program is still starting up, so there is not too
much to lose yet etc

Agreed, thanks for the assessment.

@hlein

Would you like to submit a PR for this?

@kmk3 commented on GitHub (May 7, 2025): @spiiroin > I guess the worst part is that should somebody be bitten by this deadlock, it > probably is not that easy to figure out what is going on. As the lock is > supposed to be held only very briefly, I see no harm with blocking stop for > that duration. And non-blocking lock attempts in a loop would allow sensible > diagnostic output & bailout in case of failure like this (or something > similar). At this stage the program is still starting up, so there is not too > much to lose yet etc Agreed, thanks for the assessment. @hlein Would you like to submit a PR for this?

gitea-mirror commented

2026-05-05 09:55:58 -06:00

Author

Owner

@hlein commented on GitHub (May 7, 2025):

@hlein

Would you like to submit a PR for this?

I would love to but my C is a burning trash fire. A very slow burning trash fire!

There's a chance I can assign someone to work on it in the next couple of weeks tho.

@hlein commented on GitHub (May 7, 2025): > [@hlein](https://github.com/hlein) > > Would you like to submit a PR for this? I would love to but my C is a burning trash fire. A very _slow_ burning trash fire! There's a chance I can assign someone to work on it in the next couple of weeks tho.

gitea-mirror commented

2026-05-05 09:55:59 -06:00

Author

Owner

@kmk3 commented on GitHub (May 8, 2025):

I would love to but my C is a burning trash fire. A very slow burning trash
fire!

There's a chance I can assign someone to work on it in the next couple of
weeks tho.

This seemed straightforward enough to implement, so I made an attempt with
sigprocmask, but apparently it is not possible to mask/ignore SIGSTOP:

From GNU libc.info (glibc 2.41):

25.1.3 How Signals Are Delivered

When the signal is delivered, whether right away or after a long delay, the
"specified action" for that signal is taken. For certain signals, such as
SIGKILL and SIGSTOP, the action is fixed, but for most signals, the
program has a choice: ignore the signal, specify a "handler function", or
accept the "default action" for that kind of signal. The program specifies
its choice using functions such as signal or sigaction. We sometimes say
that a handler "catches" the signal. While the handler is running, that
particular signal is normally blocked.

25.7.3 Process Signal Mask

You can't block the SIGKILL and SIGSTOP signals, but if the signal set
includes these, sigprocmask just ignores them instead of returning an error
status.

As an alternative, I thought about maybe forking into a background process just
to create the directories and then waiting on it in the main process (assuming
that background processes are not affected by ctrl+z).

Though that seems to also require exec, in which case the code in the
critical section might have to be isolated into a specific function.

Also, note that for some lock/unlock calls, the code in between them is spread
over almost 300 lines in src/firejail/main.c, so some careful refactoring might
be needed in order to reduce that:

Thoughts on this or any other ideas?

@kmk3 commented on GitHub (May 8, 2025): > I would love to but my C is a burning trash fire. A very _slow_ burning trash > fire! > > There's a chance I can assign someone to work on it in the next couple of > weeks tho. This seemed straightforward enough to implement, so I made an attempt with `sigprocmask`, but apparently it is not possible to mask/ignore `SIGSTOP`: From GNU libc.info (glibc 2.41): > 25.1.3 How Signals Are Delivered > When the signal is delivered, whether right away or after a long delay, the > "specified action" for that signal is taken. For certain signals, such as > `SIGKILL` and `SIGSTOP`, the action is fixed, but for most signals, the > program has a choice: ignore the signal, specify a "handler function", or > accept the "default action" for that kind of signal. The program specifies > its choice using functions such as `signal` or `sigaction`. We sometimes say > that a handler "catches" the signal. While the handler is running, that > particular signal is normally blocked. > 25.7.3 Process Signal Mask > You can't block the `SIGKILL` and `SIGSTOP` signals, but if the signal set > includes these, `sigprocmask` just ignores them instead of returning an error > status. As an alternative, I thought about maybe forking into a background process just to create the directories and then waiting on it in the main process (assuming that background processes are not affected by ctrl+z). Though that seems to also require `exec`, in which case the code in the critical section might have to be isolated into a specific function. Also, note that for some lock/unlock calls, the code in between them is spread over almost 300 lines in src/firejail/main.c, so some careful refactoring might be needed in order to reduce that: * <https://github.com/netblue30/firejail/blob/0d8973638cbf75a3a6d3d29a6dcc5c14541e3748/src/firejail/main.c#L3008> * <https://github.com/netblue30/firejail/blob/0d8973638cbf75a3a6d3d29a6dcc5c14541e3748/src/firejail/main.c#L3299> Thoughts on this or any other ideas?

gitea-mirror commented

2026-05-05 09:56:00 -06:00

Author

Owner

@hlein commented on GitHub (May 8, 2025):

This seemed straightforward enough to implement, so I made an attempt with sigprocmask, but apparently it is not possible to mask/ignore SIGSTOP:

Ah, I think it won't be that hard: ^Z delivers SIGSTP, not SIGSTOP. SIGSTP is what is propagated by ^Z in a shell, and it can be masked/ignored/caught.

At least, that's how I remember it. And a simple ^Z test of an strace'd process:

# strace -f -p 23791
strace: Process 23791 attached
restart_syscall(<... resuming interrupted clock_nanosleep ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGTSTP {si_signo=SIGTSTP, si_code=SI_KERNEL} ---
--- stopped by SIGTSTP ---

...This does mean that ignoring SIGSTP while the lock is held doesn't completely solve the problem - hypothetically someone could kill -STOP at just the wrong time. Although I think only root could do so, because during lockfile handling we are running as root. AFAIK SIGSTP can be propagated to a process group even if there's a privilege boundary crossed, but SIGSTOP cannot. Still, you could imagine other things going wrong while the lock is held, which is why nonblocking-busy-loop lock acquisition may still be a good idea.

Good point about examining the codepaths between lock with preproc_lock_file and unlock with preproc_unlock_file.

Also, interesting, there's both preproc_{,un}lock_firejail_dir and preproc_{,un}lock_firejail_network_dir ... and in one codepath of main() we take one lock while the other is already held:

int main(int argc, char **argv, char **envp) {
        preproc_lock_firejail_dir();
        preproc_unlock_firejail_dir();
                preproc_lock_firejail_network_dir();
        preproc_lock_firejail_dir();
        preproc_unlock_firejail_dir();
        preproc_unlock_firejail_network_dir();

I didn't look further than that yet but it may mean we need to be prepared to keep track smartly in order to restore previous signal handlers/behaviors through nested locks. Unless that can be turned into lock_network/unlock_network + lock/unlock.

@hlein commented on GitHub (May 8, 2025): > This seemed straightforward enough to implement, so I made an attempt with `sigprocmask`, but apparently it is not possible to mask/ignore `SIGSTOP`: Ah, I _think_ it won't be that hard: ^Z delivers `SIGSTP`, not `SIGSTOP`. `SIGSTP` is what is propagated by `^Z` in a shell, and it can be masked/ignored/caught. At least, that's how I remember it. And a simple ^Z test of an strace'd process: ``` # strace -f -p 23791 strace: Process 23791 attached restart_syscall(<... resuming interrupted clock_nanosleep ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal) --- SIGTSTP {si_signo=SIGTSTP, si_code=SI_KERNEL} --- --- stopped by SIGTSTP --- ``` ...This does mean that ignoring `SIGSTP` while the lock is held doesn't _completely_ solve the problem - hypothetically someone could `kill -STOP` at just the wrong time. Although I think only root could do so, because during lockfile handling we are running as root. AFAIK `SIGSTP` can be propagated to a process group even if there's a privilege boundary crossed, but `SIGSTOP` cannot. Still, you could imagine other things going wrong while the lock is held, which is why nonblocking-busy-loop lock acquisition may still be a good idea. Good point about examining the codepaths between lock with `preproc_lock_file` and unlock with `preproc_unlock_file`. Also, interesting, there's both `preproc_{,un}lock_firejail_dir` and `preproc_{,un}lock_firejail_network_dir` ... and in one codepath of `main()` we take one lock while the other is already held: ``` int main(int argc, char **argv, char **envp) { preproc_lock_firejail_dir(); preproc_unlock_firejail_dir(); preproc_lock_firejail_network_dir(); preproc_lock_firejail_dir(); preproc_unlock_firejail_dir(); preproc_unlock_firejail_network_dir(); ``` I didn't look further than that yet but it may mean we need to be prepared to keep track smartly in order to restore previous signal handlers/behaviors through nested locks. Unless that can be turned into `lock_network/unlock_network` + `lock/unlock`.

gitea-mirror commented

2026-05-05 09:56:02 -06:00

Author

Owner

@kmk3 commented on GitHub (May 9, 2025):

This seemed straightforward enough to implement, so I made an attempt with
sigprocmask, but apparently it is not possible to mask/ignore SIGSTOP:

Ah, I think it won't be that hard: ^Z delivers SIGSTP, not SIGSTOP.
SIGSTP is what is propagated by ^Z in a shell, and it can be
masked/ignored/caught.

At least, that's how I remember it. And a simple ^Z test of an strace'd
process:
# strace -f -p 23791
strace: Process 23791 attached
restart_syscall(<... resuming interrupted clock_nanosleep ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGTSTP {si_signo=SIGTSTP, si_code=SI_KERNEL} ---
--- stopped by SIGTSTP ---

Ah I see it now; it should be fine to use sigprocmask then.

The name was written multiple times as SIGSTP (without the T) rather than
SIGTSTP. I couldn't find a reference to it, so I assumed that either it was
meant as SIGSTOP or that it was just an alternative/archaic name for
SIGSTOP.

...This does mean that ignoring SIGSTP while the lock is held doesn't
completely solve the problem - hypothetically someone could kill -STOP at
just the wrong time. Although I think only root could do so, because during
lockfile handling we are running as root. AFAIK SIGSTP can be propagated to
a process group even if there's a privilege boundary crossed, but SIGSTOP
cannot. Still, you could imagine other things going wrong while the lock is
held, which is why nonblocking-busy-loop lock acquisition may still be a good
idea.

How long do you think the attempt interval should be?

Firejail seems to take anywhere from ~10ms (with --noprofile) to over 1000ms
(depending on the profile) to configure the sandbox and start the program.

Any idea how long it usually takes from the first lock to the last unlock?

Good point about examining the codepaths between lock with
preproc_lock_file and unlock with preproc_unlock_file.

Also, interesting, there's both preproc_{,un}lock_firejail_dir and
preproc_{,un}lock_firejail_network_dir ... and in one codepath of main()
we take one lock while the other is already held:
int main(int argc, char **argv, char **envp) {
        preproc_lock_firejail_dir();
        preproc_unlock_firejail_dir();
                preproc_lock_firejail_network_dir();
        preproc_lock_firejail_dir();
        preproc_unlock_firejail_dir();
        preproc_unlock_firejail_network_dir();

Indeed, I noticed that as well; not sure if it's a problem.

I didn't look further than that yet but it may mean we need to be prepared to
keep track smartly in order to restore previous signal handlers/behaviors
through nested locks. Unless that can be turned into
lock_network/unlock_network + lock/unlock.

I originally did it in a few lines by masking/unmasking SIGTSTP inside the
lock/unlock functions, though unconditionally unmasking it might be undesirable
if SIGTSTP was already masked prior to the locking. Storing the signal set
in a global variable could potentially work, but it feels like it could lead to
issues down the line if more concurrency code is introduced.

So maybe the most obvious way would be to mask SIGTSTP before calling the
first lock and restore the original mask after the last unlock.

By the way, there is already one place using sigprocmask in the code:

https://github.com/netblue30/firejail/blob/0d8973638cbf75a3a6d3d29a6dcc5c14541e3748/src/firejail/sandbox.c#L300-L338

@kmk3 commented on GitHub (May 9, 2025): > > This seemed straightforward enough to implement, so I made an attempt with > > `sigprocmask`, but apparently it is not possible to mask/ignore `SIGSTOP`: > > Ah, I _think_ it won't be that hard: ^Z delivers `SIGSTP`, not `SIGSTOP`. > `SIGSTP` is what is propagated by `^Z` in a shell, and it can be > masked/ignored/caught. > > At least, that's how I remember it. And a simple ^Z test of an strace'd > process: > > ``` > # strace -f -p 23791 > strace: Process 23791 attached > restart_syscall(<... resuming interrupted clock_nanosleep ...>) = ? ERESTART_RESTARTBLOCK (Interrupted by signal) > --- SIGTSTP {si_signo=SIGTSTP, si_code=SI_KERNEL} --- > --- stopped by SIGTSTP --- > ``` Ah I see it now; it should be fine to use `sigprocmask` then. The name was written multiple times as `SIGSTP` (without the T) rather than `SIGTSTP`. I couldn't find a reference to it, so I assumed that either it was meant as `SIGSTOP` or that it was just an alternative/archaic name for `SIGSTOP`. > ...This does mean that ignoring `SIGSTP` while the lock is held doesn't > _completely_ solve the problem - hypothetically someone could `kill -STOP` at > just the wrong time. Although I think only root could do so, because during > lockfile handling we are running as root. AFAIK `SIGSTP` can be propagated to > a process group even if there's a privilege boundary crossed, but `SIGSTOP` > cannot. Still, you could imagine other things going wrong while the lock is > held, which is why nonblocking-busy-loop lock acquisition may still be a good > idea. How long do you think the attempt interval should be? Firejail seems to take anywhere from ~10ms (with `--noprofile`) to over 1000ms (depending on the profile) to configure the sandbox and start the program. Any idea how long it usually takes from the first lock to the last unlock? > Good point about examining the codepaths between lock with > `preproc_lock_file` and unlock with `preproc_unlock_file`. > > Also, interesting, there's both `preproc_{,un}lock_firejail_dir` and > `preproc_{,un}lock_firejail_network_dir` ... and in one codepath of `main()` > we take one lock while the other is already held: > > ``` > int main(int argc, char **argv, char **envp) { > preproc_lock_firejail_dir(); > preproc_unlock_firejail_dir(); > preproc_lock_firejail_network_dir(); > preproc_lock_firejail_dir(); > preproc_unlock_firejail_dir(); > preproc_unlock_firejail_network_dir(); > ``` Indeed, I noticed that as well; not sure if it's a problem. > I didn't look further than that yet but it may mean we need to be prepared to > keep track smartly in order to restore previous signal handlers/behaviors > through nested locks. Unless that can be turned into > `lock_network/unlock_network` + `lock/unlock`. I originally did it in a few lines by masking/unmasking `SIGTSTP` inside the lock/unlock functions, though unconditionally unmasking it might be undesirable if `SIGTSTP` was already masked prior to the locking. Storing the signal set in a global variable could potentially work, but it feels like it could lead to issues down the line if more concurrency code is introduced. So maybe the most obvious way would be to mask `SIGTSTP` before calling the first lock and restore the original mask after the last unlock. By the way, there is already one place using `sigprocmask` in the code: * <https://github.com/netblue30/firejail/blob/0d8973638cbf75a3a6d3d29a6dcc5c14541e3748/src/firejail/sandbox.c#L300-L338>

gitea-mirror commented

2026-05-05 09:56:02 -06:00

Author

Owner

@hlein commented on GitHub (May 9, 2025):

Oh you're right, I can't type, s/SIGSTP/SIGTSTP/g!

I don't really know how long to expect the locking to take - we don't keep the lock for full startup, but only for some specific things. Those "specific things" probably vary by profile, but not necessarily proportional to the complexity of the app.

So Claude and I whipped up a little test harness, and I ran a few apps through it.

tl;dr: longest I ever see us holding a lock is under 8 milliseconds (on this box anyway). So we could probably busy-loop for like 20ms, then start backing off until we get to 0.5 sec or so before aborting?

# ./lock_timing.pl ls
Running 'ls' 10 times through firejail...
Run 10/10

Results after 10 iterations:
----------------------------------------

Lock hold duration statistics (20 samples):
  Mean time:   0.001814818 seconds
  Min time:    0.000041008 seconds
  Max time:    0.003677130 seconds
  Std Dev:     0.001762008 seconds
  95th %ile:   0.003677130 seconds
  99th %ile:   0.003677130 seconds

Hold duration outliers (> 3 standard deviations from mean):
  None
# ./lock_timing.pl chromium
Running 'chromium' 10 times through firejail...
Run 10/10

Results after 10 iterations:
----------------------------------------

Lock hold duration statistics (40 samples):
  Mean time:   0.000932038 seconds
  Min time:    0.000038862 seconds
  Max time:    0.003530025 seconds
  Std Dev:     0.001471662 seconds
  95th %ile:   0.003526926 seconds
  99th %ile:   0.003530025 seconds

Hold duration outliers (> 3 standard deviations from mean):
  None
# ./lock_timing.pl firejail
Running 'firejail' 10 times through firejail...
Run 10/10

Results after 10 iterations:
----------------------------------------

Lock hold duration statistics (20 samples):
  Mean time:   0.001827693 seconds
  Min time:    0.000041962 seconds
  Max time:    0.003715992 seconds
  Std Dev:     0.001773697 seconds
  95th %ile:   0.003715992 seconds
  99th %ile:   0.003715992 seconds

Hold duration outliers (> 3 standard deviations from mean):
  None
# ./lock_timing.pl steam
Running 'steam' 10 times through firejail...
Run 10/10

Results after 10 iterations:
----------------------------------------

Lock hold duration statistics (20 samples):
  Mean time:   0.001780581 seconds
  Min time:    0.000040054 seconds
  Max time:    0.003620863 seconds
  Std Dev:     0.001731405 seconds
  95th %ile:   0.003620863 seconds
  99th %ile:   0.003620863 seconds

Hold duration outliers (> 3 standard deviations from mean):
  None

Then I bumped up to 100 iterations, and ran 4 ls in parallel to potentially cause some contention; the worst of each set:

  Max time:    0.007654905 seconds
  Max time:    0.006190062 seconds
  Max time:    0.006325006 seconds
  Max time:    0.004363060 seconds

Removed the cooldown, expected to get worse results from more contention, but actually no:

  Max time:    0.004513979 seconds
  Max time:    0.004359007 seconds
  Max time:    0.004305124 seconds
  Max time:    0.005174160 seconds

Running mutt got slightly higher maxes but still below 8 ms:

  Max time:    0.007266998 seconds
  Max time:    0.004678965 seconds
  Max time:    0.007043123 seconds
  Max time:    0.006493092 seconds

Here's the script:

lock_timing.pl.txt

@hlein commented on GitHub (May 9, 2025): Oh you're right, I can't type, `s/SIGSTP/SIGTSTP/g`! I don't really know how long to expect the locking to take - we don't keep the lock for full startup, but only for some specific things. Those "specific things" probably vary by profile, but not necessarily proportional to the complexity of the app. So Claude and I whipped up a little test harness, and I ran a few apps through it. tl;dr: longest I ever see us holding a lock is under 8 milliseconds (on this box anyway). So we could probably busy-loop for like 20ms, then start backing off until we get to 0.5 sec or so before aborting? ``` # ./lock_timing.pl ls Running 'ls' 10 times through firejail... Run 10/10 Results after 10 iterations: ---------------------------------------- Lock hold duration statistics (20 samples): Mean time: 0.001814818 seconds Min time: 0.000041008 seconds Max time: 0.003677130 seconds Std Dev: 0.001762008 seconds 95th %ile: 0.003677130 seconds 99th %ile: 0.003677130 seconds Hold duration outliers (> 3 standard deviations from mean): None # ./lock_timing.pl chromium Running 'chromium' 10 times through firejail... Run 10/10 Results after 10 iterations: ---------------------------------------- Lock hold duration statistics (40 samples): Mean time: 0.000932038 seconds Min time: 0.000038862 seconds Max time: 0.003530025 seconds Std Dev: 0.001471662 seconds 95th %ile: 0.003526926 seconds 99th %ile: 0.003530025 seconds Hold duration outliers (> 3 standard deviations from mean): None # ./lock_timing.pl firejail Running 'firejail' 10 times through firejail... Run 10/10 Results after 10 iterations: ---------------------------------------- Lock hold duration statistics (20 samples): Mean time: 0.001827693 seconds Min time: 0.000041962 seconds Max time: 0.003715992 seconds Std Dev: 0.001773697 seconds 95th %ile: 0.003715992 seconds 99th %ile: 0.003715992 seconds Hold duration outliers (> 3 standard deviations from mean): None # ./lock_timing.pl steam Running 'steam' 10 times through firejail... Run 10/10 Results after 10 iterations: ---------------------------------------- Lock hold duration statistics (20 samples): Mean time: 0.001780581 seconds Min time: 0.000040054 seconds Max time: 0.003620863 seconds Std Dev: 0.001731405 seconds 95th %ile: 0.003620863 seconds 99th %ile: 0.003620863 seconds Hold duration outliers (> 3 standard deviations from mean): None ``` Then I bumped up to 100 iterations, and ran 4 ls in parallel to potentially cause some contention; the worst of each set: ``` Max time: 0.007654905 seconds Max time: 0.006190062 seconds Max time: 0.006325006 seconds Max time: 0.004363060 seconds ``` Removed the cooldown, expected to get worse results from more contention, but actually no: ``` Max time: 0.004513979 seconds Max time: 0.004359007 seconds Max time: 0.004305124 seconds Max time: 0.005174160 seconds ``` Running `mutt` got slightly higher maxes but still below 8 ms: ``` Max time: 0.007266998 seconds Max time: 0.004678965 seconds Max time: 0.007043123 seconds Max time: 0.006493092 seconds ``` Here's the script: [lock_timing.pl.txt](https://github.com/user-attachments/files/20128953/lock_timing.pl.txt)

gitea-mirror commented

2026-05-05 09:56:03 -06:00

Author

Owner

@kmk3 commented on GitHub (May 12, 2025):

I don't really know how long to expect the locking to take - we don't keep
the lock for full startup, but only for some specific things. Those "specific
things" probably vary by profile, but not necessarily proportional to the
complexity of the app.

So Claude and I whipped up a little test harness, and I ran a few apps
through it.

# ./lock_timing.pl ls
[...]

Very nice, thanks for the measurements!

tl;dr: longest I ever see us holding a lock is under 8 milliseconds (on this
box anyway). So we could probably busy-loop for like 20ms, then start backing
off until we get to 0.5 sec or so before aborting?

Sounds good.

By the way, I noticed that there is already some locking + signal handling
happening on src/firejail/main.c (set_sandbox_run_file + install_handler):

$ git grep -In \
  -e '\tinstall_handler' \
  -e preproc_ \
  -e release_sandbox_lock \
  -e set_sandbox_run_file \
  -- src/firejail/main.c
src/firejail/main.c:220:        release_sandbox_lock();
src/firejail/main.c:1181:       preproc_build_firejail_dir_unlocked();
src/firejail/main.c:1182:       preproc_lock_firejail_dir();
src/firejail/main.c:1183:       preproc_build_firejail_dir_locked();
src/firejail/main.c:1186:               preproc_clean_run();
src/firejail/main.c:1187:       preproc_unlock_firejail_dir();
src/firejail/main.c:3008:               preproc_lock_firejail_network_dir();
src/firejail/main.c:3037:       preproc_lock_firejail_dir();
src/firejail/main.c:3043:       preproc_unlock_firejail_dir();
src/firejail/main.c:3100:       set_sandbox_run_file(getpid(), child);
src/firejail/main.c:3299:       preproc_unlock_firejail_network_dir();
src/firejail/main.c:3345:       release_sandbox_lock();

Though this lock is for the instance-specific directory in
/run/firejail/profile/<PID> (using the child process PID) and the signal
handling seems to be intended just to ensure that the child process is
terminated if the parent process receives SIGINT / SIGTERM:

https://github.com/netblue30/firejail/blob/0d8973638cbf75a3a6d3d29a6dcc5c14541e3748/src/firejail/main.c#L207-L240

So maybe mask SIGTSTP before preproc_lock_firejail_dir and restore it after
release_sandbox_lock?

Though there is one potential execvp between
preproc_unlock_firejail_network_dir and release_sandbox_lock, so maybe it
would be better to restore the mask right after
preproc_unlock_firejail_network_dir.

There is also code that creates and handles child processes; not sure if the
signal masking would be an issue there.

There's a chance I can assign someone to work on it in the next couple of
weeks tho.

I'm currently working on a few other things; it would be great to have more
people working on improving the main code.

@kmk3 commented on GitHub (May 12, 2025): > I don't really know how long to expect the locking to take - we don't keep > the lock for full startup, but only for some specific things. Those "specific > things" probably vary by profile, but not necessarily proportional to the > complexity of the app. > > So Claude and I whipped up a little test harness, and I ran a few apps > through it. > ``` > # ./lock_timing.pl ls > [...] > ``` Very nice, thanks for the measurements! > tl;dr: longest I ever see us holding a lock is under 8 milliseconds (on this > box anyway). So we could probably busy-loop for like 20ms, then start backing > off until we get to 0.5 sec or so before aborting? Sounds good. By the way, I noticed that there is already some locking + signal handling happening on src/firejail/main.c (`set_sandbox_run_file` + `install_handler`): ```console $ git grep -In \ -e '\tinstall_handler' \ -e preproc_ \ -e release_sandbox_lock \ -e set_sandbox_run_file \ -- src/firejail/main.c src/firejail/main.c:220: release_sandbox_lock(); src/firejail/main.c:1181: preproc_build_firejail_dir_unlocked(); src/firejail/main.c:1182: preproc_lock_firejail_dir(); src/firejail/main.c:1183: preproc_build_firejail_dir_locked(); src/firejail/main.c:1186: preproc_clean_run(); src/firejail/main.c:1187: preproc_unlock_firejail_dir(); src/firejail/main.c:3008: preproc_lock_firejail_network_dir(); src/firejail/main.c:3037: preproc_lock_firejail_dir(); src/firejail/main.c:3043: preproc_unlock_firejail_dir(); src/firejail/main.c:3100: set_sandbox_run_file(getpid(), child); src/firejail/main.c:3299: preproc_unlock_firejail_network_dir(); src/firejail/main.c:3345: release_sandbox_lock(); ``` Though this lock is for the instance-specific directory in `/run/firejail/profile/<PID>` (using the child process PID) and the signal handling seems to be intended just to ensure that the child process is terminated if the parent process receives `SIGINT` / `SIGTERM`: * <https://github.com/netblue30/firejail/blob/0d8973638cbf75a3a6d3d29a6dcc5c14541e3748/src/firejail/main.c#L207-L240> So maybe mask `SIGTSTP` before `preproc_lock_firejail_dir` and restore it after `release_sandbox_lock`? Though there is one potential `execvp` between `preproc_unlock_firejail_network_dir` and `release_sandbox_lock`, so maybe it would be better to restore the mask right after `preproc_unlock_firejail_network_dir`. There is also code that creates and handles child processes; not sure if the signal masking would be an issue there. > There's a chance I can assign someone to work on it in the next couple of > weeks tho. I'm currently working on a few other things; it would be great to have more people working on improving the main code.

gitea-mirror commented

2026-05-05 09:56:04 -06:00

Author

Owner

@hlein commented on GitHub (May 14, 2025):

fyi @kmk3, @jlimor-kl has been working on this and will probably have a PR shortly.

@hlein commented on GitHub (May 14, 2025): fyi @kmk3, @jlimor-kl has been working on this and will probably have a PR shortly.

gitea-mirror commented

2026-05-05 09:56:04 -06:00

Author

Owner

@jlimor-kl commented on GitHub (May 15, 2025):

I have also written a first draft of the non-blocking lock + busy wait backoff for flock, would you prefer I fold this into the existing PR or wait until that one has been merged to submit a second PR?

@jlimor-kl commented on GitHub (May 15, 2025): I have also written a first draft of the non-blocking lock + busy wait backoff for flock, would you prefer I fold this into the existing PR or wait until that one has been merged to submit a second PR?

gitea-mirror commented

2026-05-05 09:56:05 -06:00

Author

Owner

@kmk3 commented on GitHub (May 16, 2025):

I have also written a first draft of the non-blocking lock + busy wait
backoff for flock, would you prefer I fold this into the existing PR or wait
until that one has been merged to submit a second PR?

To simplify the review, I think it would be better to wait for the existing PR,
as the underlying bug is trickier than usual (since it involves a race
condition) and these are 2 related but independent logical changes.

@kmk3 commented on GitHub (May 16, 2025): > I have also written a first draft of the non-blocking lock + busy wait > backoff for flock, would you prefer I fold this into the existing PR or wait > until that one has been merged to submit a second PR? To simplify the review, I think it would be better to wait for the existing PR, as the underlying bug is trickier than usual (since it involves a race condition) and these are 2 related but independent logical changes.

gitea-mirror referenced this issue

2026-05-05 10:25:35 -06:00

[PR #3348] [MERGED] Add new profile: nicotine #4732