From 4a9fccf2551ec183ccf5c085b2ab469ddf32078c Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <9erthalion6@gmail.com> Date: Tue, 17 Dec 2024 12:27:40 +0100 Subject: [PATCH] Enable clone3 (#1971) 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 --- .../workflows/integration-tests-vm-type.yml | 5 ++ collector/CMakeLists.txt | 2 +- collector/lib/CollectorConfig.h | 1 + integration-tests/container/QA_TAG | 2 +- .../container/perf_event_open/Makefile | 2 +- .../processes-listening-on-ports/Makefile | 2 +- .../container/thread_exec/Dockerfile | 18 ++++++ .../container/thread_exec/Makefile | 24 +++++++ .../container/thread_exec/thread_exec.c | 29 +++++++++ integration-tests/images.yml | 1 + integration-tests/integration_test.go | 4 ++ integration-tests/suites/threads.go | 63 +++++++++++++++++++ 12 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 integration-tests/container/thread_exec/Dockerfile create mode 100644 integration-tests/container/thread_exec/Makefile create mode 100644 integration-tests/container/thread_exec/thread_exec.c create mode 100644 integration-tests/suites/threads.go diff --git a/.github/workflows/integration-tests-vm-type.yml b/.github/workflows/integration-tests-vm-type.yml index 7a209d31da..ddf8101711 100644 --- a/.github/workflows/integration-tests-vm-type.yml +++ b/.github/workflows/integration-tests-vm-type.yml @@ -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: | diff --git a/collector/CMakeLists.txt b/collector/CMakeLists.txt index 2758ddeeee..654a896bbc 100644 --- a/collector/CMakeLists.txt +++ b/collector/CMakeLists.txt @@ -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) diff --git a/collector/lib/CollectorConfig.h b/collector/lib/CollectorConfig.h index 0bb8eaeb04..51d6602f08 100644 --- a/collector/lib/CollectorConfig.h +++ b/collector/lib/CollectorConfig.h @@ -35,6 +35,7 @@ class CollectorConfig { "accept4", "chdir", "clone", + "clone3", "close", "connect", "execve", diff --git a/integration-tests/container/QA_TAG b/integration-tests/container/QA_TAG index 227cea2156..38f77a65b3 100644 --- a/integration-tests/container/QA_TAG +++ b/integration-tests/container/QA_TAG @@ -1 +1 @@ -2.0.0 +2.0.1 diff --git a/integration-tests/container/perf_event_open/Makefile b/integration-tests/container/perf_event_open/Makefile index d48a7d4789..c1a14d3a50 100644 --- a/integration-tests/container/perf_event_open/Makefile +++ b/integration-tests/container/perf_event_open/Makefile @@ -1,4 +1,4 @@ -BASE_PAT = . +BASE_PATH = . include ../Makefile-constants.mk .DEFAULT_GOAL = all diff --git a/integration-tests/container/processes-listening-on-ports/Makefile b/integration-tests/container/processes-listening-on-ports/Makefile index ebbe90f9f5..c5de040d73 100644 --- a/integration-tests/container/processes-listening-on-ports/Makefile +++ b/integration-tests/container/processes-listening-on-ports/Makefile @@ -1,4 +1,4 @@ -BASE_PAT = . +BASE_PATH = . include ../Makefile-constants.mk .DEFAULT_GOAL = all diff --git a/integration-tests/container/thread_exec/Dockerfile b/integration-tests/container/thread_exec/Dockerfile new file mode 100644 index 0000000000..8087ec4ebd --- /dev/null +++ b/integration-tests/container/thread_exec/Dockerfile @@ -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= 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 diff --git a/integration-tests/container/thread_exec/Makefile b/integration-tests/container/thread_exec/Makefile new file mode 100644 index 0000000000..7fa19c0898 --- /dev/null +++ b/integration-tests/container/thread_exec/Makefile @@ -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) . + diff --git a/integration-tests/container/thread_exec/thread_exec.c b/integration-tests/container/thread_exec/thread_exec.c new file mode 100644 index 0000000000..b513c8af4d --- /dev/null +++ b/integration-tests/container/thread_exec/thread_exec.c @@ -0,0 +1,29 @@ +#include +#include +#include +#include + +/* + * 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); +} diff --git a/integration-tests/images.yml b/integration-tests/images.yml index a1e6df783d..f753604453 100644 --- a/integration-tests/images.yml +++ b/integration-tests/images.yml @@ -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 diff --git a/integration-tests/integration_test.go b/integration-tests/integration_test.go index c34e8977ff..44528f91a7 100644 --- a/integration-tests/integration_test.go +++ b/integration-tests/integration_test.go @@ -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)) +} diff --git a/integration-tests/suites/threads.go b/integration-tests/suites/threads.go new file mode 100644 index 0000000000..3d2a15cd78 --- /dev/null +++ b/integration-tests/suites/threads.go @@ -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) +}