From 64ac4c62abf9d89df6a6a4322bb8025902ff0ae9 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Fri, 28 Jan 2022 14:44:56 -0800 Subject: [PATCH 1/2] Define ActKillThread equal to ActKill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These constants are equal in libseccomp but Go definitions were defined separately. This resulted in dead code that never executed due to identical case statements in switch. Go can usually detect these error cases and refuses to build but for some reason this detection doesn’t work with cgo+gcc. Clang detects the equal constants correctly and therefore libseccomp-golang builds with clang broke after ActKillThread was added. In order to fix the clang build only removal of the switch case is needed. But I assumed that the setter/getter logic is supposed to work for ActKillThread as well and only way to ensure that is to set them equal like they are in C. Signed-off-by: Tonis Tiigi Signed-off-by: Sebastiaan van Stijn --- seccomp.go | 8 ++++---- seccomp_internal.go | 4 ---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/seccomp.go b/seccomp.go index e9845ed..83d52fb 100644 --- a/seccomp.go +++ b/seccomp.go @@ -202,14 +202,14 @@ const ( // This action is only usable when libseccomp API level 3 or higher is // supported. ActLog - // ActKillThread kills the thread that violated the rule. It is the same as ActKill. - // All other threads from the same thread group will continue to execute. - ActKillThread // ActKillProcess kills the process that violated the rule. // All threads in the thread group are also terminated. // This action is only usable when libseccomp API level 3 or higher is // supported. ActKillProcess + // ActKillThread kills the thread that violated the rule. It is the same as ActKill. + // All other threads from the same thread group will continue to execute. + ActKillThread = ActKill ) const ( @@ -385,7 +385,7 @@ func (a ScmpCompareOp) String() string { // String returns a string representation of a seccomp match action func (a ScmpAction) String() string { switch a & 0xFFFF { - case ActKill, ActKillThread: + case ActKillThread: return "Action: Kill thread" case ActKillProcess: return "Action: Kill process" diff --git a/seccomp_internal.go b/seccomp_internal.go index dae9a79..40161c9 100644 --- a/seccomp_internal.go +++ b/seccomp_internal.go @@ -629,8 +629,6 @@ func (a ScmpCompareOp) toNative() C.int { func actionFromNative(a C.uint32_t) (ScmpAction, error) { aTmp := a & 0xFFFF switch a & 0xFFFF0000 { - case C.C_ACT_KILL: - return ActKill, nil case C.C_ACT_KILL_PROCESS: return ActKillProcess, nil case C.C_ACT_KILL_THREAD: @@ -655,8 +653,6 @@ func actionFromNative(a C.uint32_t) (ScmpAction, error) { // Only use with sanitized actions, no error handling func (a ScmpAction) toNative() C.uint32_t { switch a & 0xFFFF { - case ActKill: - return C.C_ACT_KILL case ActKillProcess: return C.C_ACT_KILL_PROCESS case ActKillThread: From 598bc8245f09ce963e7268705f6f5cffc56381b8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 22 Apr 2022 12:02:06 +0200 Subject: [PATCH 2/2] Deprecate ActKill in favor of ActKillThread Signed-off-by: Sebastiaan van Stijn --- seccomp.go | 10 ++++++---- seccomp_internal.go | 2 +- seccomp_test.go | 14 +++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/seccomp.go b/seccomp.go index 83d52fb..8dad12f 100644 --- a/seccomp.go +++ b/seccomp.go @@ -182,9 +182,9 @@ const ( // ActInvalid is a placeholder to ensure uninitialized ScmpAction // variables are invalid ActInvalid ScmpAction = iota - // ActKill kills the thread that violated the rule. It is the same as ActKillThread. + // ActKillThread kills the thread that violated the rule. // All other threads from the same thread group will continue to execute. - ActKill + ActKillThread // ActTrap throws SIGSYS ActTrap // ActNotify triggers a userspace notification. This action is only usable when @@ -207,9 +207,11 @@ const ( // This action is only usable when libseccomp API level 3 or higher is // supported. ActKillProcess - // ActKillThread kills the thread that violated the rule. It is the same as ActKill. + // ActKill kills the thread that violated the rule. // All other threads from the same thread group will continue to execute. - ActKillThread = ActKill + // + // Deprecated: use ActKillThread + ActKill = ActKillThread ) const ( diff --git a/seccomp_internal.go b/seccomp_internal.go index 40161c9..df4dfb7 100644 --- a/seccomp_internal.go +++ b/seccomp_internal.go @@ -293,7 +293,7 @@ const ( archStart ScmpArch = ArchNative archEnd ScmpArch = ArchRISCV64 // Comparison boundaries to check for action validity - actionStart ScmpAction = ActKill + actionStart ScmpAction = ActKillThread actionEnd ScmpAction = ActKillProcess // Comparison boundaries to check for comparison operator validity compareOpStart ScmpCompareOp = CompareNotEqual diff --git a/seccomp_test.go b/seccomp_test.go index 7fb5e4b..c65dbf0 100644 --- a/seccomp_test.go +++ b/seccomp_test.go @@ -274,7 +274,7 @@ func TestFilterCreateRelease(t *testing.T) { t.Errorf("Can create filter with invalid action") } - filter, err := NewFilter(ActKill) + filter, err := NewFilter(ActKillThread) if err != nil { t.Errorf("Error creating filter: %s", err) } @@ -291,7 +291,7 @@ func TestFilterCreateRelease(t *testing.T) { } func TestFilterReset(t *testing.T) { - filter, err := NewFilter(ActKill) + filter, err := NewFilter(ActKillThread) if err != nil { t.Errorf("Error creating filter: %s", err) } @@ -301,7 +301,7 @@ func TestFilterReset(t *testing.T) { action, err := filter.GetDefaultAction() if err != nil { t.Errorf("Error getting default action of filter") - } else if action != ActKill { + } else if action != ActKillThread { t.Errorf("Default action of filter was set incorrectly!") } @@ -326,7 +326,7 @@ func TestFilterReset(t *testing.T) { } func TestFilterArchFunctions(t *testing.T) { - filter, err := NewFilter(ActKill) + filter, err := NewFilter(ActKillThread) if err != nil { t.Errorf("Error creating filter: %s", err) } @@ -402,7 +402,7 @@ func TestFilterArchFunctions(t *testing.T) { } func TestFilterAttributeGettersAndSetters(t *testing.T) { - filter, err := NewFilter(ActKill) + filter, err := NewFilter(ActKillThread) if err != nil { t.Errorf("Error creating filter: %s", err) } @@ -411,7 +411,7 @@ func TestFilterAttributeGettersAndSetters(t *testing.T) { act, err := filter.GetDefaultAction() if err != nil { t.Errorf("Error getting default action: %s", err) - } else if act != ActKill { + } else if act != ActKillThread { t.Errorf("Default action was set incorrectly") } @@ -547,7 +547,7 @@ func TestMergeFilters(t *testing.T) { t.Errorf("Source filter should not be valid after merging") } - filter3, err := NewFilter(ActKill) + filter3, err := NewFilter(ActKillThread) if err != nil { t.Errorf("Error creating filter: %s", err) }