From 7b0dfb0cdbdcb402c000d30399dbfd2eeebe1266 Mon Sep 17 00:00:00 2001
From: Fabricio Voznika <fvoznika@google.com>
Date: Fri, 24 Aug 2018 11:38:12 -0700
Subject: SyscallRules merge and add were dropping AllowAny rules

PiperOrigin-RevId: 210131001
Change-Id: I285707c5143b3e4c9a6948c1d1a452b6f16e65b7
---
 pkg/seccomp/seccomp_rules.go | 17 ++++++++++++---
 pkg/seccomp/seccomp_test.go  | 52 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/pkg/seccomp/seccomp_rules.go b/pkg/seccomp/seccomp_rules.go
index 892ccabb4..4b99792fd 100644
--- a/pkg/seccomp/seccomp_rules.go
+++ b/pkg/seccomp/seccomp_rules.go
@@ -34,7 +34,7 @@ func seccompDataOffsetArgLow(i int) uint32 {
 }
 
 func seccompDataOffsetArgHigh(i int) uint32 {
-	return uint32(seccompDataOffsetArgs + i*8 + 4)
+	return seccompDataOffsetArgLow(i) + 4
 }
 
 // AllowAny is marker to indicate any value will be accepted.
@@ -100,7 +100,11 @@ func NewSyscallRules() SyscallRules {
 // AddRule adds the given rule. It will create a new entry for a new syscall, otherwise
 // it will append to the existing rules.
 func (sr SyscallRules) AddRule(sysno uintptr, r Rule) {
-	if _, ok := sr[sysno]; ok {
+	if cur, ok := sr[sysno]; ok {
+		// An empty rules means allow all. Honor it when more rules are added.
+		if len(cur) == 0 {
+			sr[sysno] = append(sr[sysno], Rule{})
+		}
 		sr[sysno] = append(sr[sysno], r)
 	} else {
 		sr[sysno] = []Rule{r}
@@ -110,7 +114,14 @@ func (sr SyscallRules) AddRule(sysno uintptr, r Rule) {
 // Merge merges the given SyscallRules.
 func (sr SyscallRules) Merge(rules SyscallRules) {
 	for sysno, rs := range rules {
-		if _, ok := sr[sysno]; ok {
+		if cur, ok := sr[sysno]; ok {
+			// An empty rules means allow all. Honor it when more rules are added.
+			if len(cur) == 0 {
+				sr[sysno] = append(sr[sysno], Rule{})
+			}
+			if len(rs) == 0 {
+				rs = []Rule{Rule{}}
+			}
 			sr[sysno] = append(sr[sysno], rs...)
 		} else {
 			sr[sysno] = rs
diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go
index d3aca7ee9..9f9507228 100644
--- a/pkg/seccomp/seccomp_test.go
+++ b/pkg/seccomp/seccomp_test.go
@@ -355,3 +355,55 @@ func TestRealDeal(t *testing.T) {
 		}
 	}
 }
+
+// TestMerge ensures that empty rules are not erased when rules are merged.
+func TestMerge(t *testing.T) {
+	for _, tst := range []struct {
+		name  string
+		main  []Rule
+		merge []Rule
+		want  []Rule
+	}{
+		{
+			name:  "empty both",
+			main:  nil,
+			merge: nil,
+			want:  []Rule{Rule{}, Rule{}},
+		},
+		{
+			name:  "empty main",
+			main:  nil,
+			merge: []Rule{Rule{}},
+			want:  []Rule{Rule{}, Rule{}},
+		},
+		{
+			name:  "empty merge",
+			main:  []Rule{Rule{}},
+			merge: nil,
+			want:  []Rule{Rule{}, Rule{}},
+		},
+	} {
+		t.Run(tst.name, func(t *testing.T) {
+			mainRules := SyscallRules{1: tst.main}
+			mergeRules := SyscallRules{1: tst.merge}
+			mainRules.Merge(mergeRules)
+			if got, want := len(mainRules[1]), len(tst.want); got != want {
+				t.Errorf("wrong length, got: %d, want: %d", got, want)
+			}
+			for i, r := range mainRules[1] {
+				if r != tst.want[i] {
+					t.Errorf("result, got: %v, want: %v", r, tst.want[i])
+				}
+			}
+		})
+	}
+}
+
+// TestAddRule ensures that empty rules are not erased when rules are added.
+func TestAddRule(t *testing.T) {
+	rules := SyscallRules{1: {}}
+	rules.AddRule(1, Rule{})
+	if got, want := len(rules[1]), 2; got != want {
+		t.Errorf("len(rules[1]), got: %d, want: %d", got, want)
+	}
+}
-- 
cgit v1.2.3