Skip to content

Commit

Permalink
Enable clone3 (#1971)
Browse files Browse the repository at this point in the history
Originally we've excluded clone3, which wasn't present on some older
platforms. But now it's getting used by newer glibc, which means that
we're missing information about threads. It plays some visible role only
when threads are exiting, e.g. in a weird situation if a thread will do
exec, all the threads in the thread group are going to be stopped,
leaving only one running exec. This is visible in the resolved file path
for the event.

Along the way add few more improvements:
* Fix type with BASE_PATH
* Switch log level if debugging the run
  • Loading branch information
erthalion authored Dec 17, 2024
1 parent 9abd6f8 commit 4a9fccf
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/integration-tests-vm-type.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ jobs:
run: |
make -C "${{ github.workspace }}/ansible" create-ci-vms
- name: Set log level
if: ${{ runner.debug == '1' }}
run: |
echo "COLLECTOR_LOG_LEVEL=trace" >> "$GITHUB_ENV"
- name: Run Tests
if: ${{ ! inputs.run-benchmarks }}
run: |
Expand Down
2 changes: 1 addition & 1 deletion collector/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,6 @@ set(SCAP_HOST_ROOT_ENV_VAR_NAME "COLLECTOR_HOST_ROOT" CACHE STRING "Host root en
set(BUILD_LIBSCAP_MODERN_BPF ON CACHE BOOL "Enable modern bpf engine" FORCE)
set(MODERN_BPF_DEBUG_MODE ${BPF_DEBUG_MODE} CACHE BOOL "Enable BPF debug prints" FORCE)

set(MODERN_BPF_EXCLUDE_PROGS "^(openat2|ppoll|setsockopt|clone3|io_uring_setup|nanosleep)$" CACHE STRING "Set of syscalls to exclude from modern bpf engine " FORCE)
set(MODERN_BPF_EXCLUDE_PROGS "^(openat2|ppoll|setsockopt|io_uring_setup|nanosleep)$" CACHE STRING "Set of syscalls to exclude from modern bpf engine " FORCE)

add_subdirectory(${FALCO_DIR} falco)
1 change: 1 addition & 0 deletions collector/lib/CollectorConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class CollectorConfig {
"accept4",
"chdir",
"clone",
"clone3",
"close",
"connect",
"execve",
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/container/QA_TAG
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.0.0
2.0.1
2 changes: 1 addition & 1 deletion integration-tests/container/perf_event_open/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
BASE_PAT = .
BASE_PATH = .
include ../Makefile-constants.mk

.DEFAULT_GOAL = all
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
BASE_PAT = .
BASE_PATH = .
include ../Makefile-constants.mk

.DEFAULT_GOAL = all
Expand Down
18 changes: 18 additions & 0 deletions integration-tests/container/thread_exec/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
FROM ubuntu:jammy

COPY thread_exec.c /thread_exec.c

RUN apt update -y && apt install gcc -y && \
gcc -lpthread thread_exec.c -o /usr/bin/thread_exec

# XXX: s390x reports task_comm with a leading slash if the entrypoint
# will have it:
#
# [TRACE] (Service.cpp:156) /thread_exec (52684) < execve res=0
# exe=/thread_exec args=NULL tid=52684(/thread_exec) pid=52684(/thread_exec)
# ptid=52664(sh) cwd=<NA> comm=/thread_exec trusted_exepath=/thread_exec ...
#
# It looks like we don't exercise anything similar in other tests, so just make
# sure the binary is in PATH for now.

ENTRYPOINT thread_exec
24 changes: 24 additions & 0 deletions integration-tests/container/thread_exec/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
BASE_PATH = .
include ../Makefile-constants.mk

.DEFAULT_GOAL = all

COLLECTOR_QA_THREAD_EXEC := collector-thread-exec

ifneq ($(COLLECTOR_QA_TAG),)
COLLECTOR_QA_THREAD_EXEC=collector-thread-exec-$(COLLECTOR_QA_TAG)
endif

.PHONY: all
all: build

.PHONY: build
build:
@docker buildx build --load --platform $(PLATFORM) \
-t quay.io/rhacs-eng/qa-multi-arch:$(COLLECTOR_QA_THREAD_EXEC) .

.PHONY: build-and-push
build-and-push:
@docker buildx build --push --platform $(PLATFORM) \
-t quay.io/rhacs-eng/qa-multi-arch:$(COLLECTOR_QA_THREAD_EXEC) .

29 changes: 29 additions & 0 deletions integration-tests/container/thread_exec/thread_exec.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

/*
* Spawn a thread, then exec an arbitrary binary from it. Depending on glibc
* version, this will produce clone + exec or clone3 + exec. Exec from a thread
* will tear down any other threads and reassign the thread group leader pid to
* this thread (it's nicely explained in "map ptrace", section "execve(2) under
* ptrace". This should cause threads cleanup logic in Falco, and produce
* visible effect: the recorder process should have "thread_exec" file path,
* rather than ls (coreutils).
*/

void* threadTest(void* vargp) {
sleep(5);
char* argument_list[] = {"/bin/ls", NULL};
execvp(*argument_list, argument_list);
return NULL;
}

int main() {
pthread_t thread_id;
pthread_create(&thread_id, NULL, threadTest, NULL);
while (1) {
};
exit(0);
}
1 change: 1 addition & 0 deletions integration-tests/images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ qa:
qa-alpine-curl: quay.io/rhacs-eng/qa-multi-arch:alpine-curl
qa-perf-event-open: quay.io/rhacs-eng/qa-multi-arch:collector-perf-event-open
qa-udp: quay.io/rhacs-eng/qa-multi-arch:udp
qa-thread-exec: quay.io/rhacs-eng/qa-multi-arch:collector-thread-exec

non_qa:
nginx: nginx:1.14-alpine
4 changes: 4 additions & 0 deletions integration-tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,7 @@ func TestUdpNetworkFlow(t *testing.T) {
func TestRuntimeConfigFile(t *testing.T) {
suite.Run(t, new(suites.RuntimeConfigFileTestSuite))
}

func TestThreads(t *testing.T) {
suite.Run(t, new(suites.ThreadsTestSuite))
}
63 changes: 63 additions & 0 deletions integration-tests/suites/threads.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package suites

import (
"fmt"
"time"

"github.com/stackrox/collector/integration-tests/pkg/common"
"github.com/stackrox/collector/integration-tests/pkg/config"
"github.com/stackrox/collector/integration-tests/pkg/types"
"github.com/stretchr/testify/assert"
)

type ThreadsTestSuite struct {
IntegrationTestSuiteBase
}

func (s *ThreadsTestSuite) SetupSuite() {
s.RegisterCleanup("thread-exec")
s.StartCollector(false, nil)
}

func (s *ThreadsTestSuite) TearDownSuite() {
s.StopCollector()
s.cleanupContainers("thread-exec")
}

// Verify that Collector correctly traces threads, even if created via clone3.
// This should lead to a correct file path, when doing exec from a thread --
// instead of an exec target we should see the parent file path.
func (s *ThreadsTestSuite) TestThreadExec() {
image := config.Images().QaImageByKey("qa-thread-exec")
containerID, err := s.Executor().StartContainer(
config.ContainerStartConfig{
Name: "thread-exec",
Image: image,
})
s.Require().NoError(err)

if finished, _ := s.waitForContainerToExit("thread-exec", containerID, 10*time.Second, 0); finished {
expectedProcesses := []types.ProcessInfo{
types.ProcessInfo{
Name: "thread_exec",
ExePath: "/usr/bin/thread_exec",
Uid: 0,
Gid: 0,
Args: "",
},
}

s.Sensor().ExpectProcesses(s.T(), common.ContainerShortID(containerID),
10*time.Second, expectedProcesses...)

logs, err := s.containerLogs("perf-event-open")
if err != nil {
fmt.Println(logs)
}

} else {
assert.FailNow(s.T(), "Timeout waiting for thread-exec")
}

s.cleanupContainers(containerID)
}

0 comments on commit 4a9fccf

Please sign in to comment.