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

[1.1] seccomp: patchbpf: always include native architecture in stub #4391

Merged

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Sep 3, 2024

Backport of #4219 and #3635.


It turns out that on ppc64le (at least), Docker doesn't include any
architectures in the list of allowed architectures. libseccomp
interprets this as "just include the default architecture" but patchbpf
would return a no-op ENOSYS stub, which would lead to the exact issues
that commit 7a8d716 ("seccomp: prepend -ENOSYS stub to all
filters") fixed for other architectures.

So, just always include the running architecture in the list. There's
no real downside.

Ref: https://bugzilla.suse.com/show_bug.cgi?id=1192051#c6
Reported-by: Fabian Vogt fvogt@suse.com
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

kolyshkin and others added 3 commits September 3, 2024 13:13
(This is a cherry-pick of 2cd05e4.)

In findLastSyscalls, we convert libseccomp.ArchNative to the real
libseccomp architecture, but archToNative already does that, so
this code is redundant.

Remove the redundant code, and move its comment to archToNative.

Fixes: 7a8d716 ("seccomp: prepend -ENOSYS stub to all filters")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a backport of b288abe.)

Calling the Linux AUDIT_* architecture constants "native" leads to
confusing code when we are getting the actual native architecture of the
running system.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a backport of ccc500c.)

It turns out that on ppc64le (at least), Docker doesn't include any
architectures in the list of allowed architectures. libseccomp
interprets this as "just include the default architecture" but patchbpf
would return a no-op ENOSYS stub, which would lead to the exact issues
that commit 7a8d716 ("seccomp: prepend -ENOSYS stub to all
filters") fixed for other architectures.

So, just always include the running architecture in the list. There's
no real downside.

Ref: https://bugzilla.suse.com/show_bug.cgi?id=1192051#c6
Fixes: 7a8d716 ("seccomp: prepend -ENOSYS stub to all filters")
Reported-by: Fabian Vogt <fvogt@suse.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
@cyphar cyphar added area/seccomp backport/1.1-pr A backport to 1.1.x release. labels Sep 3, 2024
@cyphar
Copy link
Member Author

cyphar commented Sep 3, 2024

I forgot to include this for 1.1.14 (we have backports of this for the SLES runc package). 😅

@cyphar cyphar added this to the 1.1.15 milestone Sep 3, 2024
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +235 to +238
// On architectures like ppc64le, Docker inexplicably doesn't include the
// native architecture in the architecture list which results in no
// architectures being present in the list at all (rendering the ENOSYS
// stub a no-op). So, always include the native architecture.
Copy link
Member

Choose a reason for hiding this comment

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

Can we report this to upstream docker? (besides keeping this code, of course)

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened moby/moby#48471. It's a bit light on the details but I linked to the SUSE bug where I debugged this earlier this year.

@cyphar cyphar merged commit 3216d3b into opencontainers:release-1.1 Sep 10, 2024
28 checks passed
@cyphar cyphar deleted the 1.1-seccomp-patchbpf-ppc64le branch September 10, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/seccomp backport/1.1-pr A backport to 1.1.x release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants