Skip to content

Commit 618e149

Browse files
committed
[1.1] seccomp: patchbpf: always include native architecture in stub
(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 <[email protected]> Signed-off-by: Aleksa Sarai <[email protected]>
1 parent d85b583 commit 618e149

File tree

2 files changed

+61
-8
lines changed

2 files changed

+61
-8
lines changed

libcontainer/seccomp/patchbpf/enosys_linux.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,16 +224,30 @@ type lastSyscallMap map[linuxAuditArch]map[libseccomp.ScmpArch]libseccomp.ScmpSy
224224
// representation, but SCMP_ARCH_X32 means we have to track cases where the
225225
// same architecture has different largest syscalls based on the mode.
226226
func findLastSyscalls(config *configs.Seccomp) (lastSyscallMap, error) {
227-
lastSyscalls := make(lastSyscallMap)
228-
// Only loop over architectures which are present in the filter. Any other
229-
// architectures will get the libseccomp bad architecture action anyway.
227+
scmpArchs := make(map[libseccomp.ScmpArch]struct{})
230228
for _, ociArch := range config.Architectures {
231229
arch, err := libseccomp.GetArchFromString(ociArch)
232230
if err != nil {
233231
return nil, fmt.Errorf("unable to validate seccomp architecture: %w", err)
234232
}
233+
scmpArchs[arch] = struct{}{}
234+
}
235+
// On architectures like ppc64le, Docker inexplicably doesn't include the
236+
// native architecture in the architecture list which results in no
237+
// architectures being present in the list at all (rendering the ENOSYS
238+
// stub a no-op). So, always include the native architecture.
239+
if nativeScmpArch, err := libseccomp.GetNativeArch(); err != nil {
240+
return nil, fmt.Errorf("unable to get native arch: %w", err)
241+
} else if _, ok := scmpArchs[nativeScmpArch]; !ok {
242+
logrus.Debugf("seccomp: adding implied native architecture %v to config set", nativeScmpArch)
243+
scmpArchs[nativeScmpArch] = struct{}{}
244+
}
245+
logrus.Debugf("seccomp: configured architecture set: %s", scmpArchs)
235246

236-
// Figure out native architecture representation of the architecture.
247+
// Only loop over architectures which are present in the filter. Any other
248+
// architectures will get the libseccomp bad architecture action anyway.
249+
lastSyscalls := make(lastSyscallMap)
250+
for arch := range scmpArchs {
237251
auditArch, err := scmpArchToAuditArch(arch)
238252
if err != nil {
239253
return nil, fmt.Errorf("cannot map architecture %v to AUDIT_ARCH_ constant: %w", arch, err)

libcontainer/seccomp/patchbpf/enosys_linux_test.go

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/opencontainers/runc/libcontainer/configs"
1313

1414
libseccomp "github.com/seccomp/libseccomp-golang"
15+
"github.com/sirupsen/logrus"
1516
"golang.org/x/net/bpf"
1617
)
1718

@@ -105,6 +106,18 @@ var testArches = []string{
105106
"ppc64le",
106107
"s390",
107108
"s390x",
109+
// Dummy value to indicate a configuration with no architecture specified.
110+
"native",
111+
}
112+
113+
var nativeArch string
114+
115+
func init() {
116+
scmpNativeArch, err := libseccomp.GetNativeArch()
117+
if err != nil {
118+
logrus.Panicf("get native arch: %v", err)
119+
}
120+
nativeArch = scmpNativeArch.String()
108121
}
109122

110123
func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string) {
@@ -155,6 +168,9 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
155168
expected uint32
156169
}
157170

171+
if arch == "native" {
172+
arch = nativeArch
173+
}
158174
scmpArch, err := libseccomp.GetArchFromString(arch)
159175
if err != nil {
160176
t.Fatalf("unknown libseccomp architecture %q: %v", arch, err)
@@ -228,8 +244,15 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
228244

229245
// Test syscalls in the explicit list.
230246
for _, test := range syscallTests {
231-
// Override the expected value in the two special cases.
232-
if !archSet[arch] || isAllowAction(defaultAction) {
247+
// Override the expected value in the two special cases:
248+
// 1. If the default action is allow, the filter won't have
249+
// the stub prepended so we expect a fallthrough.
250+
// 2. If the executing architecture is not in the architecture
251+
// set, then the architecture is not handled by the stub --
252+
// *except* in the case of the native architecture (which
253+
// is always included in the stub).
254+
if isAllowAction(defaultAction) ||
255+
(!archSet[arch] && arch != nativeArch) {
233256
test.expected = retFallthrough
234257
}
235258

@@ -263,7 +286,14 @@ var testActions = map[string]configs.Action{
263286

264287
func TestEnosysStub_SingleArch(t *testing.T) {
265288
for _, arch := range testArches {
266-
arches := []string{arch}
289+
var arches []string
290+
// "native" indicates a blank architecture field for seccomp, to test
291+
// the case where the running architecture was not included in the
292+
// architecture. Docker doesn't always set the architecture for some
293+
// reason (namely for ppc64le).
294+
if arch != "native" {
295+
arches = append(arches, arch)
296+
}
267297
t.Run("arch="+arch, func(t *testing.T) {
268298
for name, action := range testActions {
269299
t.Run("action="+name, func(t *testing.T) {
@@ -277,7 +307,16 @@ func TestEnosysStub_SingleArch(t *testing.T) {
277307
func TestEnosysStub_MultiArch(t *testing.T) {
278308
for end := 0; end < len(testArches); end++ {
279309
for start := 0; start < end; start++ {
280-
arches := testArches[start:end]
310+
var arches []string
311+
for _, arch := range testArches[start:end] {
312+
// "native" indicates a blank architecture field for seccomp, to test
313+
// the case where the running architecture was not included in the
314+
// architecture. Docker doesn't always set the architecture for some
315+
// reason (namely for ppc64le).
316+
if arch != "native" {
317+
arches = append(arches, arch)
318+
}
319+
}
281320
if len(arches) <= 1 {
282321
continue
283322
}

0 commit comments

Comments
 (0)