Use RET_KILL_PROCESS if available in kernel

RET_KILL_THREAD doesn't work well for Go because it will
kill only the offending thread and leave the process hanging.
RET_TRAP can be masked out and it's not guaranteed to kill
the process. RET_KILL_PROCESS is available since 4.14.

For older kernel, continue to use RET_TRAP as this is the
best option (likely to kill process, easy to debug).

PiperOrigin-RevId: 222357867
Change-Id: Icc1d7d731274b16c2125b7a1ba4f7883fbdb2cbd

4 files changed, 83 insertions, 21 deletions

diff --git a/pkg/seccomp/seccomp.go b/pkg/seccomp/seccomp.go
index 1dfbf749e..9d714d02d 100644
--- a/pkg/seccomp/seccomp.go
+++ b/pkg/seccomp/seccomp.go
@@ -33,17 +33,42 @@ const (
 	defaultLabel = "default_action"
 )
 
+func actionName(a uint32) string {
+	switch a {
+	case linux.SECCOMP_RET_KILL_PROCESS:
+		return "kill process"
+	case linux.SECCOMP_RET_TRAP:
+		return "trap"
+	}
+	panic(fmt.Sprintf("invalid action: %d", a))
+}
+
 // Install generates BPF code based on the set of syscalls provided. It only
-// allows syscalls that conform to the specification and generates SIGSYS
-// trap unless kill is set.
+// allows syscalls that conform to the specification. Syscalls that violate the
+// specification will trigger RET_KILL_PROCESS, except for the cases below.
+//
+// RET_TRAP is used in violations, instead of RET_KILL_PROCESS, in the
+// following cases:
+//	 1. Kernel doesn't support RET_KILL_PROCESS: RET_KILL_THREAD only kills the
+//      offending thread and often keeps the sentry hanging.
+//   2. Debug: RET_TRAP generates a panic followed by a stack trace which is
+//      much easier to debug then RET_KILL_PROCESS which can't be caught.
 //
-// This is a convenience wrapper around BuildProgram and SetFilter.
-func Install(rules SyscallRules, kill bool) error {
-	log.Infof("Installing seccomp filters for %d syscalls (kill=%t)", len(rules), kill)
-	defaultAction := uint32(linux.SECCOMP_RET_TRAP)
-	if kill {
-		defaultAction = uint32(linux.SECCOMP_RET_KILL)
+// Be aware that RET_TRAP sends SIGSYS to the process and it may be ignored,
+// making it possible for the process to continue running after a violation.
+// However, it will leave a SECCOMP audit event trail behind. In any case, the
+// syscall is still blocked from executing.
+func Install(rules SyscallRules) error {
+	defaultAction, err := defaultAction()
+	if err != nil {
+		return err
 	}
+
+	// Uncomment to get stack trace when there is a violation.
+	// defaultAction = uint32(linux.SECCOMP_RET_TRAP)
+
+	log.Infof("Installing seccomp filters for %d syscalls (action=%s)", len(rules), actionName(defaultAction))
+
 	instrs, err := BuildProgram([]RuleSet{
 		RuleSet{
 			Rules:  rules,
@@ -70,6 +95,17 @@ func Install(rules SyscallRules, kill bool) error {
 	return nil
 }
 
+func defaultAction() (uint32, error) {
+	available, err := isKillProcessAvailable()
+	if err != nil {
+		return 0, err
+	}
+	if available {
+		return uint32(linux.SECCOMP_RET_KILL_PROCESS), nil
+	}
+	return uint32(linux.SECCOMP_RET_TRAP), nil
+}
+
 // RuleSet is a set of rules and associated action.
 type RuleSet struct {
 	Rules  SyscallRules
diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go
index 226f30b7b..f2b903e42 100644
--- a/pkg/seccomp/seccomp_test.go
+++ b/pkg/seccomp/seccomp_test.go
@@ -121,7 +121,7 @@ func TestBasic(t *testing.T) {
 					Action: linux.SECCOMP_RET_TRAP,
 				},
 			},
-			defaultAction: linux.SECCOMP_RET_KILL,
+			defaultAction: linux.SECCOMP_RET_KILL_THREAD,
 			specs: []spec{
 				{
 					desc: "Multiple rulesets allowed (1a)",
@@ -141,7 +141,7 @@ func TestBasic(t *testing.T) {
 				{
 					desc: "Multiple rulesets allowed (2)",
 					data: seccompData{nr: 0, arch: linux.AUDIT_ARCH_X86_64},
-					want: linux.SECCOMP_RET_KILL,
+					want: linux.SECCOMP_RET_KILL_THREAD,
 				},
 			},
 		},
@@ -431,15 +431,23 @@ func TestRealDeal(t *testing.T) {
 				t.Errorf("victim was not killed as expected, output: %s", out)
 				continue
 			}
+			// Depending on kernel version, either RET_TRAP or RET_KILL_PROCESS is
+			// used. RET_TRAP dumps reason for exit in output, while RET_KILL_PROCESS
+			// returns SIGSYS as exit status.
+			if !strings.Contains(string(out), test.want) &&
+				!strings.Contains(err.Error(), test.want) {
+				t.Errorf("Victim error is wrong, got: %v, err: %v, want: %v", string(out), err, test.want)
+				continue
+			}
 		} else {
 			if err != nil {
 				t.Errorf("victim failed to execute, err: %v", err)
 				continue
 			}
-		}
-		if !strings.Contains(string(out), test.want) {
-			t.Errorf("Victim output is wrong, got: %v, want: %v", err, test.want)
-			continue
+			if !strings.Contains(string(out), test.want) {
+				t.Errorf("Victim output is wrong, got: %v, want: %v", string(out), test.want)
+				continue
+			}
 		}
 	}
 }
diff --git a/pkg/seccomp/seccomp_test_victim.go b/pkg/seccomp/seccomp_test_victim.go
index 007038273..dd5ed0041 100644
--- a/pkg/seccomp/seccomp_test_victim.go
+++ b/pkg/seccomp/seccomp_test_victim.go
@@ -106,7 +106,7 @@ func main() {
 		}
 	}
 
-	if err := seccomp.Install(syscalls, false); err != nil {
+	if err := seccomp.Install(syscalls); err != nil {
 		fmt.Printf("Failed to install seccomp: %v", err)
 		os.Exit(1)
 	}
diff --git a/pkg/seccomp/seccomp_unsafe.go b/pkg/seccomp/seccomp_unsafe.go
index dd009221a..a31c6471d 100644
--- a/pkg/seccomp/seccomp_unsafe.go
+++ b/pkg/seccomp/seccomp_unsafe.go
@@ -36,22 +36,40 @@ type sockFprog struct {
 //
 //go:nosplit
 func SetFilter(instrs []linux.BPFInstruction) syscall.Errno {
-	// SYS_SECCOMP is not available in syscall package.
-	const SYS_SECCOMP = 317
-
 	// PR_SET_NO_NEW_PRIVS is required in order to enable seccomp. See seccomp(2) for details.
 	if _, _, errno := syscall.RawSyscall(syscall.SYS_PRCTL, linux.PR_SET_NO_NEW_PRIVS, 1, 0); errno != 0 {
 		return errno
 	}
 
-	// TODO: Use SECCOMP_FILTER_FLAG_KILL_PROCESS when available.
 	sockProg := sockFprog{
 		Len:    uint16(len(instrs)),
 		Filter: (*linux.BPFInstruction)(unsafe.Pointer(&instrs[0])),
 	}
-	if _, _, errno := syscall.RawSyscall(SYS_SECCOMP, linux.SECCOMP_SET_MODE_FILTER, linux.SECCOMP_FILTER_FLAG_TSYNC, uintptr(unsafe.Pointer(&sockProg))); errno != 0 {
-		return errno
+	return seccomp(linux.SECCOMP_SET_MODE_FILTER, linux.SECCOMP_FILTER_FLAG_TSYNC, unsafe.Pointer(&sockProg))
+}
+
+func isKillProcessAvailable() (bool, error) {
+	action := uint32(linux.SECCOMP_RET_KILL_PROCESS)
+	if errno := seccomp(linux.SECCOMP_GET_ACTION_AVAIL, 0, unsafe.Pointer(&action)); errno != 0 {
+		// EINVAL: SECCOMP_GET_ACTION_AVAIL not in this kernel yet.
+		// EOPNOTSUPP: SECCOMP_RET_KILL_PROCESS not supported.
+		if errno == syscall.EINVAL || errno == syscall.EOPNOTSUPP {
+			return false, nil
+		}
+		return false, errno
 	}
+	return true, nil
+}
 
+// seccomp calls seccomp(2). This is safe to call from an afterFork context.
+//
+//go:nosplit
+func seccomp(op, flags uint32, ptr unsafe.Pointer) syscall.Errno {
+	// SYS_SECCOMP is not available in syscall package.
+	const SYS_SECCOMP = 317
+
+	if _, _, errno := syscall.RawSyscall(SYS_SECCOMP, uintptr(op), uintptr(flags), uintptr(ptr)); errno != 0 {
+		return errno
+	}
 	return 0
 }