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

Enable --seccomp-bpf by default #274

Closed
i-ky opened this issue Nov 12, 2023 · 2 comments
Closed

Enable --seccomp-bpf by default #274

i-ky opened this issue Nov 12, 2023 · 2 comments
Labels

Comments

@i-ky
Copy link

i-ky commented Nov 12, 2023

As mentioned in sunlin7/compile-db-gen#2, adding --seccomp-bpf can result in a huge performance improvement. Why not enable it by default?

I understand, that there may be corner cases, when --seccomp-bpf has undesirable effects. But the majority of use cases can really benefit from it. For applications/scripts making use of strace and aiming to be compatible across different strace versions it may be too difficult to figure out if --seccomp-bpf is supported. It would be very nice to make this decision on strace side.

@ldv-alt
Copy link
Member

ldv-alt commented Jun 3, 2024

--seccomp-bpf option that enables seccomp assisted syscall filtering also implies --kill-on-exit option which instructs strace to apply PTRACE_O_EXITKILL ptrace option to all tracee processes (which sends a SIGKILL signal to the tracee if the tracer exits) and not to detach them on cleanup so they will not be left running after the tracer exit.

This was made to ensure that strace won't leave any processes with a seccomp program installed for syscall filtering purposes. For the same reason --seccomp-bpf requires --follow-forks and is not compatible with --attach=pid option.

This --kill-on-exit behaviour is not the default because it diverges from the traditional strace behavior that allowed tracees to be detached, and there are valid workflows based on that behavior.

If that's not enough, a tracer process will not be notified if another seccomp filter returns an action value with a precedence greater than SECCOMP_RET_TRACE. That is, if another seccomp filter, for example, disabled the syscall or even killed the process, strace --seccomp-bpf won't be notified about that syscall invocation at all.

On top of this, clone(CLONE_UNTRACED) disables PTRACE_EVENT_{CLONE|FORK|VFORK} events, and in absence of CLONE_PTRACE it also disables tracing of the child process. To workaround this, strace would have to strip CLONE_UNTRACED flag in each invocation of clone syscall, and emulate it instead.

Sorry for being a bit technical here, but here we are.
In other words, we are not even remotely close to enabling --seccomp-bpf by default.
We provide this option in hope it would be useful, but enabling it remains to be a conscious choice made by the user.

@ldv-alt ldv-alt closed this as completed Jun 3, 2024
ldv-alt added a commit that referenced this issue Jun 4, 2024
* doc/strace.1.in (--seccomp-bpf): Document potential side effects of
another seccomp filter that returns an action value with a precedence
greater than SECCOMP_RET_TRACE.

References: #274
@esyr
Copy link
Member

esyr commented Jun 4, 2024

On top of this, clone(CLONE_UNTRACED) disables PTRACE_EVENT_{CLONE|FORK|VFORK} events, and in absence of CLONE_PTRACE it also disables tracing of the child process. To workaround this, strace would have to strip CLONE_UNTRACED flag in each invocation of clone syscall, and emulate it instead.

Filed as #303.

ldv-alt added a commit that referenced this issue Jun 4, 2024
* doc/strace.1.in (--seccomp-bpf): Document potential side effects of
another seccomp filter that returns an action value with a precedence
greater than SECCOMP_RET_TRACE.

References: #274
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants