all: remove SetTsync()

Now that we don't allow seccomp notify without tsync, we require
apiLevel 6 (simultaneous use of SCMP_FLTATR_CTL_TSYNC and the notify
APIs.)

Also, revert documentation changes to NewFilter to remove reference to
SetTsync().

Signed-off-by: Rodrigo Campos <[email protected]>
Acked-by: Tom Hromatka <[email protected]>
Signed-off-by: Paul Moore <[email protected]>

Author: rata

Makefile

@@ -24,17 +24,8 @@ vet:
 # be noticed earlier in the CI.
 TEST_TIMEOUT=10s
 
-# Some tests run with SetTsync(false) and some tests with SetTsync(true). Once
-# the threads are not using the same seccomp filters anymore, the kernel will
-# refuse to use Tsync, causing next tests to fail. This issue could be left
-# unnoticed if the test with SetTsync(false) is executed last.
-#
-# Run tests twice ensure that no test leave the testing process in a state
-# unable to run following tests, regardless of the subset of tests selected.
-TEST_COUNT=2
-
 test:
-	go test -v -timeout $(TEST_TIMEOUT) -count $(TEST_COUNT)
+	go test -v -timeout $(TEST_TIMEOUT)
 
 lint:
 	@$(if $(shell which golint),true,$(error "install golint and include it in your PATH"))

seccomp.go

@@ -200,7 +200,7 @@ const (
 	// ActTrap throws SIGSYS
 	ActTrap ScmpAction = iota
 	// ActNotify triggers a userspace notification. This action is only usable when
-	// libseccomp API level 5 or higher is supported.
+	// libseccomp API level 6 or higher is supported.
 	ActNotify ScmpAction = iota
 	// ActErrno causes the syscall to return a negative error code. This
 	// code can be set with the SetReturnCode method
@@ -604,9 +604,7 @@ type ScmpFilter struct {
 }
 
 // NewFilter creates and returns a new filter context.  Accepts a default action to be
-// taken for syscalls which match no rules in the filter. The newly created filter applies
-// to all threads of the calling process. Use SetTsync() to change this behavior prior to
-// loading the filter.
+// taken for syscalls which match no rules in the filter.
 // Returns a reference to a valid filter context, or nil and an error
 // if the filter context could not be created or an invalid default action was given.
 func NewFilter(defaultAction ScmpAction) (*ScmpFilter, error) {
@@ -638,27 +636,6 @@ func NewFilter(defaultAction ScmpAction) (*ScmpFilter, error) {
 	return filter, nil
 }
 
-// SetTsync sets or clears the filter's thread-sync (TSYNC) attribute. When set, this attribute
-// tells the kernel to synchronize all threads of the calling process to the same seccomp filter.
-// When using filters with the seccomp notification action (ActNotify), the TSYNC attribute
-// must be cleared prior to loading the filter. Refer to the seccomp manual page (seccomp(2)) for
-// further details.
-func (f *ScmpFilter) SetTsync(val bool) error {
-	var cval C.uint32_t
-
-	if val == true {
-		cval = 1
-	} else {
-		cval = 0
-	}
-
-	err := f.setFilterAttr(filterAttrTsync, cval)
-	if err != nil && val == false && err == syscall.ENOTSUP {
-		return nil
-	}
-	return err
-}
-
 // IsValid determines whether a filter context is valid to use.
 // Some operations (Release and Merge) render filter contexts invalid and
 // consequently prevent further use.

seccomp_internal.go

@@ -737,8 +737,8 @@ func (f *ScmpFilter) getNotifFd() (ScmpFd, error) {
 
 	// Ignore error, if not supported returns apiLevel == 0
 	apiLevel, _ := GetAPI()
-	if apiLevel < 5 {
-		return -1, fmt.Errorf("seccomp notification requires API level >= 5; current level = %d", apiLevel)
+	if apiLevel < 6 {
+		return -1, fmt.Errorf("seccomp notification requires API level >= 6; current level = %d", apiLevel)
 	}
 
 	fd := C.seccomp_notify_fd(f.filterCtx)
@@ -752,8 +752,8 @@ func notifReceive(fd ScmpFd) (*ScmpNotifReq, error) {
 
 	// Ignore error, if not supported returns apiLevel == 0
 	apiLevel, _ := GetAPI()
-	if apiLevel < 5 {
-		return nil, fmt.Errorf("seccomp notification requires API level >= 5; current level = %d", apiLevel)
+	if apiLevel < 6 {
+		return nil, fmt.Errorf("seccomp notification requires API level >= 6; current level = %d", apiLevel)
 	}
 
 	// we only use the request here; the response is unused
@@ -778,8 +778,8 @@ func notifRespond(fd ScmpFd, scmpResp *ScmpNotifResp) error {
 
 	// Ignore error, if not supported returns apiLevel == 0
 	apiLevel, _ := GetAPI()
-	if apiLevel < 5 {
-		return fmt.Errorf("seccomp notification requires API level >= 5; current level = %d", apiLevel)
+	if apiLevel < 6 {
+		return fmt.Errorf("seccomp notification requires API level >= 6; current level = %d", apiLevel)
 	}
 
 	// we only use the reponse here; the request is discarded
@@ -803,8 +803,8 @@ func notifRespond(fd ScmpFd, scmpResp *ScmpNotifResp) error {
 func notifIDValid(fd ScmpFd, id uint64) error {
 	// Ignore error, if not supported returns apiLevel == 0
 	apiLevel, _ := GetAPI()
-	if apiLevel < 5 {
-		return fmt.Errorf("seccomp notification requires API level >= 5; current level = %d", apiLevel)
+	if apiLevel < 6 {
+		return fmt.Errorf("seccomp notification requires API level >= 6; current level = %d", apiLevel)
 	}
 
 	if retCode := C.seccomp_notify_id_valid(C.int(fd), C.uint64_t(id)); retCode != 0 {

seccomp_test.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"runtime"
 	"strings"
 	"syscall"
 	"testing"
@@ -843,7 +842,7 @@ func TestNotif(t *testing.T) {
 	execInSubprocess(t, subprocessNotif)
 }
 func subprocessNotif(t *testing.T) {
-	// seccomp notification requires API level >= 5
+	// seccomp notification requires API level >= 6
 	api, err := GetAPI()
 	if err != nil {
 		if !APILevelIsSupported() {
@@ -853,10 +852,10 @@ func subprocessNotif(t *testing.T) {
 		t.Errorf("Error getting API level: %s", err)
 	} else {
 		t.Logf("Got API level %v", api)
-		if api < 5 {
-			err = SetAPI(5)
+		if api < 6 {
+			err = SetAPI(6)
 			if err != nil {
-				t.Skipf("Skipping test: API level %d is less than 5 and could not set it to 5", api)
+				t.Skipf("Skipping test: API level %d is less than 6 and could not set it to 6", api)
 				return
 			}
 		}
@@ -874,18 +873,16 @@ func subprocessNotif(t *testing.T) {
 		return
 	}
 
+	// Create a filter that only notifies on chdir. This way, while the
+	// seccomp filter applies to all threads, we can run the target and
+	// handling in different go routines with no problem as only the target
+	// goroutine uses chdir.
 	filter, err := NewFilter(ActAllow)
 	if err != nil {
 		t.Errorf("Error creating filter: %s", err)
 	}
 	defer filter.Release()
 
-	// Seccomp notification is only supported on single-thread filters
-	err = filter.SetTsync(false)
-	if err != nil {
-		t.Errorf("Error setting tsync on filter: %s", err)
-	}
-
 	call, err := GetSyscallFromName("chdir")
 	if err != nil {
 		t.Errorf("Error getting syscall number: %s", err)
@@ -954,11 +951,6 @@ func subprocessNotif(t *testing.T) {
 	done := make(chan struct{})
 
 	go func() {
-		// Lock this goroutine to it's current kernel thread; otherwise the go runtime may
-		// switch us to a different OS thread, bypassing the seccomp notification filter.
-		runtime.LockOSThread()
-		defer runtime.UnlockOSThread()
-
 		err = filter.Load()
 		if err != nil {
 			t.Errorf("Error loading filter: %s", err)
@@ -1017,14 +1009,14 @@ func TestNotifUnsupported(t *testing.T) {
 	execInSubprocess(t, subprocessNotifUnsupported)
 }
 func subprocessNotifUnsupported(t *testing.T) {
-	// seccomp notification requires API level >= 5
+	// seccomp notification requires API level >= 6
 	api := 0
 	if APILevelIsSupported() {
 		api, err := GetAPI()
 		if err != nil {
 			t.Errorf("Error getting API level: %s", err)
-		} else if api >= 5 {
-			t.Skipf("Skipping test for old libseccomp support: API level %d is >= 5", api)
+		} else if api >= 6 {
+			t.Skipf("Skipping test for old libseccomp support: API level %d is >= 6", api)
 		}
 	}