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

32-bit related seccomp issues #2783

Closed
kolyshkin opened this issue Feb 2, 2021 · 5 comments
Closed

32-bit related seccomp issues #2783

kolyshkin opened this issue Feb 2, 2021 · 5 comments

Comments

@kolyshkin
Copy link
Contributor

kolyshkin commented Feb 2, 2021

I finally managed to be able to run i386-cross unit test (see #2768) and got these two issues:

=== RUN   TestHostDevicesAllValid
--- PASS: TestHostDevicesAllValid (0.00s)
PASS
ok  	github.com/opencontainers/runc/libcontainer/devices	0.008s
# github.com/opencontainers/runc/libcontainer/seccomp/patchbpf [github.com/opencontainers/runc/libcontainer/seccomp/patchbpf.test]
libcontainer/seccomp/patchbpf/enosys_linux_test.go:180:20: constant 3735928559 overflows int
libcontainer/seccomp/patchbpf/enosys_linux_test.go:204:19: constant 3735928559 overflows int
libcontainer/seccomp/patchbpf/enosys_linux_test.go:227:25: constant 3735928559 overflows int
=== RUN   TestUsernsCheckpoint
--- PASS: TestUsernsCheckpoint (0.57s)

and the second one (caused by using x86_64 images under i386, now fixed):
...

=== RUN TestSeccompDenyGetcwdWithErrno
seccomp_test.go:83: Expected output pwd: getcwd: No such process but got
--- FAIL: TestSeccompDenyGetcwdWithErrno (0.37s)
=== RUN TestSeccompDenyGetcwd
seccomp_test.go:152: Expected output pwd: getcwd: Operation not permitted but got
--- FAIL: TestSeccompDenyGetcwd (0.32s)
=== RUN TestSeccompPermitWriteConditional
seccomp_test.go:207: signal: bad system call (core dumped):
--- FAIL: TestSeccompPermitWriteConditional (0.31s)
=== RUN TestSeccompDenyWriteConditional
--- PASS: TestSeccompDenyWriteConditional (0.33s)
=== RUN TestSeccompPermitWriteMultipleConditions
seccomp_test.go:332: |: signal: bad system call (core dumped)
--- FAIL: TestSeccompPermitWriteMultipleConditions (0.31s)
=== RUN TestSeccompDenyWriteMultipleConditions
--- PASS: TestSeccompDenyWriteMultipleConditions (0.33s)
=== RUN TestSeccompMultipleConditionSameArgDeniesStdout
seccomp_test.go:437: |: signal: bad system call (core dumped)
--- FAIL: TestSeccompMultipleConditionSameArgDeniesStdout (0.33s)
=== RUN TestSeccompMultipleConditionSameArgDeniesStderr
--- PASS: TestSeccompMultipleConditionSameArgDeniesStderr (0.31s)
FAIL
FAIL github.com/opencontainers/runc/libcontainer/integration 9.887s

@cyphar
Copy link
Member

cyphar commented Feb 2, 2021

This is because golang.org/x/net/bpf returns an int from their emulated BPF VM implementation when they should really be returning uint32. I'll send a patch to fix it, though since it's a breaking change they may not like it. However this patch should fix the immediate issue:

diff --git a/libcontainer/seccomp/patchbpf/enosys_linux_test.go b/libcontainer/seccomp/patchbpf/enosys_linux_test.go
index 17b92af95867..1675c6eae6d4 100644
--- a/libcontainer/seccomp/patchbpf/enosys_linux_test.go
+++ b/libcontainer/seccomp/patchbpf/enosys_linux_test.go
@@ -159,7 +159,7 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
                        type syscallTest struct {
                                syscall  string
                                sysno    libseccomp.ScmpSyscall
-                               expected int
+                               expected uint32
                        }
 
                        scmpArch, err := libseccomp.GetArchFromString(arch)
@@ -177,9 +177,9 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
                        // Add explicit syscalls (whether they will return -ENOSYS
                        // depends on the filter rules).
                        for idx, syscall := range explicitSyscalls {
-                               expected := int(retFallthrough)
+                               expected := retFallthrough
                                if idx >= enosysStart {
-                                       expected = int(retErrnoEnosys)
+                                       expected = retErrnoEnosys
                                }
                                sysno, err := libseccomp.GetSyscallFromNameByArch(syscall, scmpArch)
                                if err != nil {
@@ -201,7 +201,7 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
                                syscallTests = append(syscallTests, syscallTest{
                                        sysno:    sysno,
                                        syscall:  syscall,
-                                       expected: int(retFallthrough),
+                                       expected: retFallthrough,
                                })
                        }
 
@@ -216,7 +216,7 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
                                syscallTests = append(syscallTests, syscallTest{
                                        sysno:    sysno,
                                        syscall:  fmt.Sprintf("syscall_%#x", sysno),
-                                       expected: int(retErrnoEnosys),
+                                       expected: retErrnoEnosys,
                                })
                        }
 
@@ -224,14 +224,17 @@ func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string)
                        for _, test := range syscallTests {
                                // Override the expected value in the two special cases.
                                if !archSet[arch] || isAllowAction(defaultAction) {
-                                       test.expected = int(retFallthrough)
+                                       test.expected = retFallthrough
                                }
 
                                payload := mockSyscallPayload(t, test.sysno, nativeArch, 0x1337, 0xF00BA5)
-                               ret, err := filter.Run(payload)
+                               // NOTE: golang.org/x/net/bpf returns int here rather than
+                               //       uint32. See <>.
+                               rawRet, err := filter.Run(payload)
                                if err != nil {
                                        t.Fatalf("error running filter: %v", err)
                                }
+                               ret := uint32(rawRet)
                                if ret != test.expected {
                                        t.Logf("mock filter for %v %v:", arches, allowedSyscalls)
                                        for idx, insn := range program {

@kolyshkin
Copy link
Contributor Author

The second issue is probably caused by running x86_64 container under i386. Looking...

@kolyshkin
Copy link
Contributor Author

The second issue is probably caused by running x86_64 container under i386. Looking...

Fixed by rebasing on top of #2741

@kolyshkin
Copy link
Contributor Author

However this patch should fix the immediate issue

@cyphar initially I just changed 0xDEADBEEF to 0xBADBEEF to verify that things work, but then I took your diff.

@kolyshkin
Copy link
Contributor Author

47a9649

I guess we can close this one now.

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

No branches or pull requests

2 participants