bugfix: fix startup race condition for /run/firejail directory #6307
Conversation
|
From Saifish OS patch here: sailfishos/firejail#18 |
|
Systemd is using temporary directories with random namse (not predictable and unlike to collide), like |
kmk3
changed the title
AFAIK the difference is that here the directories are meant to be shared and thus must have predictable known names i.e. the problem was not accidental collisions but a technical glitches on the way to the desired end result. |
spiiroin
force-pushed
the
serialize_remounts
branch
from
b8d9a11
to
36ff43c
Compare
kmk3
changed the title
There was a problem hiding this comment.
I made a few more changes and pushed it to a serialize_remounts_v2
branch.
Main changes:
- Fix misc formatting in the code and commit message
- Make lock variables global and use them directly only in preproc.c
- Use the lock/unlock functions also for the network lock (for consistency)
- Log if the path is already locked/unlocked
- Split the original commit into one for the flock improvements and another for
the main fix (to make it easier to understand the scope of the latter)
If it seems good, you can hard-reset to that branch, add the SOB trailer to the
commits if you want and force-push. Let me know if you have any questions.
(By the way, note that git trailers cannot have blank lines between them; a
blank line is what separates the trailers from the rest of the message).
To enable using them outside of src/firejail/main.c.
spiiroin
force-pushed
the
serialize_remounts
branch
from
36ff43c
to
025b145
Compare
Was fine by me, took the branch as-is (plus rebase as there were already new commits in the master branch). |
Misc: On future PRs, please do a separate force-push before or after rebasing |
Changes: * Centralize flock handling in preproc.c * Add debug and error logging * Abort if anything fails Co-authored-by: Kelvin M. Klann <[email protected]>
There are reports of firejail sandboxed applications occasionally taking a long time (12 seconds) to start up. When this happens, it affects all sandboxed applications until the device is rebooted. The reason for the slowdown seems to be a timing hazard in the way remounts under /run/firejail are handled. This gets triggered when multiple firejail processes are launched in parallel as part of user session bring up and results in some, dozens, hundreds, or even thousands of stray /run/firejail/xxx mounts. The amount of mount points then affects every mount operation that is done during sandbox filesystem construction. To stop this from happening, arrange it so that only one firejail process at time is inspecting and/or modifying mountpoints under /run/firejail by doing: 1. Create /run/firejail directory (without locking) 2. Create and obtain a lock for /run/firejail/firejail-run.lock 3. Setup files, directories and mounts under /run/firejail 4. Release /run/firejail/firejail-run.lock
kmk3
force-pushed
the
serialize_remounts
branch
from
025b145
to
e53b6d6
Compare
kmk3
changed the title
There are reports of firejail sandboxed applications occasionally taking long time (12 seconds) to start up. When this happens, it affects all sandboxed applications until the device is rebooted.
The reason for the slowdown seems to be a timing hazard in the way remounts under /run/firejail are handled. This gets triggered when multiple firejail processes are launched in parallel as part of user session bring up and results in some, dozens, hundreds, or even thousands of stray /run/firejail/xxx mounts. The amount of mount points then affects every mount operation that is done during sandbox filesystem construction.
To stop this from happening, arrange it so that only one firejail process at time is inspecting and/or modifying mountpoints under /run/firejail by doing: