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

Allow killing process rather than thread with seccomp #11967

Closed
bobrik opened this issue Mar 12, 2019 · 9 comments
Closed

Allow killing process rather than thread with seccomp #11967

bobrik opened this issue Mar 12, 2019 · 9 comments
Labels
Milestone

Comments

@bobrik
Copy link

bobrik commented Mar 12, 2019

Is your feature request related to a problem? Please describe.

I'm frustrated when programs make syscalls they should not.

I've tried using SystemCallFilter with a Go program and seccomp killed a thread silently, blackholing some part of the program, but otherwise keeping it running: golang/go#30753.

Describe the solution you'd like

I want to be able to kill whole process rather than thread. Currently SCMP_ACT_KILL is used:

Seccomp also allows SCMP_ACT_KILL_PROCESS:

       SCMP_ACT_KILL_PROCESS
              The entire process will be terminated by the kernel with
              SIGSYS when it calls a syscall that does not match any of the
              configured seccomp filter rules.

It seems like SCMP_ACT_TRAP and SCMP_ACT_LOG may be useful as well.

@bobrik
Copy link
Author

bobrik commented Mar 12, 2019

It may be reasonable to switch to SCMP_ACT_KILL_PROCESS by default as well.

@dqminh
Copy link
Contributor

dqminh commented Mar 12, 2019

Looking at this, one workaround is to set SystemCallErrorNumber=EPERM which doesnt terminate the thread but instead returning EPERM, which should be more user-friendly than killing the thread without a catchable signal.

I'm also not sure that SCMP_ACT_KILL is a suitable default. That seems like it will be very non-obvious for a lot of programs ( golang for example where it expect a catchable SIGSYS in order to panic properly ). What do we think about making SCMP_ACT_ERRNO(EPERM) the default, and have another knob to customize the seccomp action. SystemCallErrorAction=allow|trap|kill|trace|errno|log(for new kernel ) ?

cc @poettering

@poettering
Copy link
Member

We really should never use SCMP_ACT_KILL at all. Sounds like a historic mistake, and we should ust change that. killing individual threads is never OK, it should always be the full thing.

I think it would be great if we could have a new option SystemCallFiltreLog= or so which takes a boolean and allows logging of the syscalls independently of denying/allowing them.

@poettering
Copy link
Member

hmm, on my system SCMP_ACT_KILL_PROCESS doesn't exist yet, unfortunately :-(

Kernel support was added in 0466bdb99e8744bc9befa8d62a317f0fd7fd7421(Aug 2017), but my libseccomp version still doesn't have it (f29)...

I figure we should change our logic to pick SCMP_ACT_KILL_PROCESS dynamically if libseccomp and kernel support it, and otherwise continue to use SCMP_ACT_KILL even if it sucks for threaded apps...

@poettering poettering added this to the v242 milestone Mar 12, 2019
@bobrik
Copy link
Author

bobrik commented Mar 12, 2019

Commit adding support for SCMP_ACT_KILL_PROCESS in libseccomp:

It's not in any released version yet.

@poettering
Copy link
Member

Hmm, I guess we have to wait until they release a version...

@bobrik
Copy link
Author

bobrik commented Mar 12, 2019

Good point, I asked for a release: seccomp/libseccomp#145

@poettering
Copy link
Member

Moving to to v243 milestone, since libseccomp doesn't expose this yet.

@poettering poettering modified the milestones: v242, v243 Mar 12, 2019
poettering added a commit to poettering/systemd that referenced this issue Apr 29, 2019
If we have it, use it. It makes a ton more sense.

Fixes: systemd#11967
@poettering
Copy link
Member

Fix waiting in #12430

poettering added a commit to poettering/systemd that referenced this issue Apr 30, 2019
If we have it, use it. It makes a ton more sense.

Fixes: systemd#11967
poettering added a commit to poettering/systemd that referenced this issue Apr 30, 2019
If we have it, use it. It makes a ton more sense.

Fixes: systemd#11967
edevolder pushed a commit to edevolder/systemd that referenced this issue Jun 26, 2019
If we have it, use it. It makes a ton more sense.

Fixes: systemd#11967
bobrik added a commit to bobrik/seccomp that referenced this issue Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants