Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deterministic-shutdown option #4635

Merged
merged 1 commit into from
Nov 13, 2021
Merged

Conversation

smitsohu
Copy link
Collaborator

Sometimes sandboxed apps are outlived by their child processes which prevents the sandbox from shutting down.

Add a new option to deal with this situation.

Probably there are better names for the option, I just needed a placeholder. kill-remains was suggested by @msva.

Tracked in #4440
Closes #928
Closes #3042

Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Voting against noorphans because the no prefix suggests that it is an hardening option. Maybe deterministic-shutdown? Does it terminate/kill child-child processes or does it exit will the child-child processes are still running?


Reminder:

If you add a new command, here's the checklist:

  • Update manpages: firejail(1) and firejail-profile(5)
  • Update shell completions
  • Update vim syntax files
  • Update --help

@smitsohu
Copy link
Collaborator Author

smitsohu commented Oct 24, 2021

It forcefully shuts down the sandbox if the oldest child in the sandbox dies. Or in other words, it forcefully shuts down the sandbox if the first program, the one that was started by Firejail, dies.

The sandbox is shut down even if another process has joined the sandbox, which is a bit of an inconsistency but difficult to avoid.

Reminder:

Will do, but it will take a while, I'm very slow on these things.

@smitsohu
Copy link
Collaborator Author

The sandbox is shut down even if another process has joined the sandbox, which is a bit of an inconsistency but difficult to avoid.

Now that I'm writing this down it dawns on me that this is not playing nice with join-or-start.

@smitsohu
Copy link
Collaborator Author

Now that I'm writing this down it dawns on me that this is not playing nice with join-or-start.

On the other hand nothing bad will happen. Maybe there is even a use case for
"join-or-start into one sandbox, but kill everything if first app dies"
so documenting should be enough.

@smitsohu
Copy link
Collaborator Author

I guess I like deterministic-shutdown.

It is less violent than everything I came up with (kill-children, drop-orphans, ...)

@kmk3
Copy link
Collaborator

kmk3 commented Oct 26, 2021

@smitsohu commented on Oct 26:

I guess I like deterministic-shutdown.

It is less violent than everything I came up with (kill-children,
drop-orphans, ...)

A few that had come to mind were kill-all-<x> and kill-<x>-on-exit, though
they don't sound very nice either.

drop-orphans takes the cake though; I can only imagine it being used out of
context:

Firejail is not closing for some reason.

Did you remember to drop-orphans?

@smitsohu smitsohu changed the title noorphans option deterministic-shutdown option Oct 26, 2021
@smitsohu
Copy link
Collaborator Author

Ok, I added a stub to the man pages. I still need to think about if or how to expand it.

@smitsohu smitsohu marked this pull request as ready for review October 26, 2021 21:16
@netblue30 netblue30 merged commit 02b9b93 into netblue30:master Nov 13, 2021
@netblue30
Copy link
Owner

all in!

@KOLANICH
Copy link
Contributor

Thanks for implementing it.

kmk3 added a commit that referenced this pull request Dec 11, 2021
@smitsohu smitsohu deleted the noorphans branch January 12, 2022 19:18
@kmk3 kmk3 added the enhancement New feature request label Jan 27, 2022
JoelLinn added a commit to JoelLinn/compiler-explorer that referenced this pull request Jul 10, 2022
- replace `--terminate-orphans` with `--deterministic-shutdown` as it
  was renamed in the upstream PR:
  netblue30/firejail#4635
JoelLinn added a commit to JoelLinn/compiler-explorer that referenced this pull request Jul 10, 2022
- replace `--terminate-orphans` with `--deterministic-shutdown` as it
  was renamed in the upstream PR:
  netblue30/firejail#4635
apmorton pushed a commit to compiler-explorer/compiler-explorer that referenced this pull request Aug 23, 2022
- replace `--terminate-orphans` with `--deterministic-shutdown` as it
  was renamed in the upstream PR:
  netblue30/firejail#4635
apmorton added a commit to compiler-explorer/compiler-explorer that referenced this pull request Aug 28, 2022
* Update firejail path

* Fix firejail command options

- replace `--terminate-orphans` with `--deterministic-shutdown` as it
  was renamed in the upstream PR:
  netblue30/firejail#4635

Co-authored-by: Joel Linn <jl@conductive.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
Status: Done (on RELNOTES)
5 participants