From be8a04bb71bcadd699b358c9f7c55c50caba804c Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 24 Jan 2025 16:07:13 +0100 Subject: [PATCH 1/3] processmanager: make ProcessPIDExit private The recommended way to report new mappings or exited process SynchronizeProcess() should be used. Signed-off-by: Florian Lehner --- processmanager/manager_test.go | 2 +- processmanager/processinfo.go | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/processmanager/manager_test.go b/processmanager/manager_test.go index b4515d258..76589b35f 100644 --- a/processmanager/manager_test.go +++ b/processmanager/manager_test.go @@ -602,7 +602,7 @@ func TestProcExit(t *testing.T) { populateManager(t, manager) - manager.ProcessPIDExit(testcase.pid) + manager.processPIDExit(testcase.pid) assert.Equal(t, testcase.deletePidPageMappingCount, ebpfMockup.deletePidPageMappingCount) assert.Equal(t, testcase.deleteStackDeltaRangesCount, diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 41b087fa5..b874963d9 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -507,12 +507,11 @@ func (pm *ProcessManager) synchronizeMappings(pr process.Process, return newProcess } -// ProcessPIDExit informs the ProcessManager that a process exited and no longer will be scheduled. +// processPIDExit informs the ProcessManager that a process exited and no longer will be scheduled. // exitKTime is stored for later processing in ProcessedUntil, when traces up to this time have been // processed. There can be a race condition if we can not clean up the references for this process // fast enough and this particular pid is reused again by the system. -// NOTE: Exported only for tracer. -func (pm *ProcessManager) ProcessPIDExit(pid libpf.PID) { +func (pm *ProcessManager) processPIDExit(pid libpf.PID) { exitKTime := times.GetKTime() log.Debugf("- PID: %v", pid) defer pm.ebpf.RemoveReportedPID(pid) @@ -520,24 +519,22 @@ func (pm *ProcessManager) ProcessPIDExit(pid libpf.PID) { pm.mu.Lock() defer pm.mu.Unlock() - pidExitProcessed := false info, pidExists := pm.pidToProcessInfo[pid] if pidExists || (pm.interpreterTracerEnabled && len(pm.interpreters[pid]) > 0) { // ProcessPIDExit may be called multiple times in short succession // for the same PID, don't update exitKTime if we've previously recorded it. - if _, pidExitProcessed = pm.exitEvents[pid]; !pidExitProcessed { + if _, pidExitProcessed := pm.exitEvents[pid]; !pidExitProcessed { pm.exitEvents[pid] = exitKTime + } else { + log.Debugf("Skip duplicate process exit handling for PID %d", pid) + return } } if !pidExists { log.Debugf("Skip process exit handling for unknown PID %d", pid) return } - if pidExitProcessed { - log.Debugf("Skip duplicate process exit handling for PID %d", pid) - return - } // Delete all entries we have for this particular PID from pid_page_to_mapping_info. deleted, err := pm.ebpf.DeletePidPageMappingInfo(pid, []lpm.Prefix{dummyPrefix}) @@ -555,6 +552,8 @@ func (pm *ProcessManager) ProcessPIDExit(pid libpf.PID) { } } +// SynchronizeProcess triggers ProcessManager to update its internal information +// about a process. This includes process exit information as well as changed memory mappings. func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pid := pr.PID() log.Debugf("= PID: %v", pid) @@ -576,7 +575,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { // All other errors imply that the process has exited. // Clean up, and notify eBPF. - pm.ProcessPIDExit(pid) + pm.processPIDExit(pid) if os.IsNotExist(err) { // Since listing /proc and opening files in there later is inherently racy, // we expect to lose the race sometimes and thus expect to hit os.IsNotExist. @@ -602,7 +601,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { // the process is changing: all mappings, comm, etc. If execve fails, we // reaped it early. If execve succeeds, we will get new synchronization // request soon, and handle it as a new process event. - pm.ProcessPIDExit(pid) + pm.processPIDExit(pid) return } @@ -640,7 +639,7 @@ func (pm *ProcessManager) CleanupPIDs() { pm.mu.RUnlock() for _, pid := range deadPids { - pm.ProcessPIDExit(pid) + pm.processPIDExit(pid) } if len(deadPids) > 0 { From 9575f3d8c8fecee1c807b68f1d47d21589d4c9f5 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 24 Jan 2025 16:25:01 +0100 Subject: [PATCH 2/3] processmanager: return early in processPIDExit() Return early from processPIDExit() if pm.pidToProcessInfo is not aware of a paritcular PID. This early return is possible, as processPIDExit() holds the global ProcessManager lock and entries in pm.pidToProcessInfo, pm.interpreters and pm.exitEvents are cleared in ProcessedUntil() while holding the lock. Signed-off-by: Florian Lehner --- processmanager/processinfo.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index b874963d9..1553d21c4 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -520,6 +520,10 @@ func (pm *ProcessManager) processPIDExit(pid libpf.PID) { defer pm.mu.Unlock() info, pidExists := pm.pidToProcessInfo[pid] + if !pidExists { + log.Debugf("Skip process exit handling for unknown PID %d", pid) + return + } if pidExists || (pm.interpreterTracerEnabled && len(pm.interpreters[pid]) > 0) { // ProcessPIDExit may be called multiple times in short succession @@ -531,10 +535,6 @@ func (pm *ProcessManager) processPIDExit(pid libpf.PID) { return } } - if !pidExists { - log.Debugf("Skip process exit handling for unknown PID %d", pid) - return - } // Delete all entries we have for this particular PID from pid_page_to_mapping_info. deleted, err := pm.ebpf.DeletePidPageMappingInfo(pid, []lpm.Prefix{dummyPrefix}) From a66ec6361f2bf55e7687b8bd3d84747c6d5bd13a Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 24 Jan 2025 16:35:50 +0100 Subject: [PATCH 3/3] Update processmanager/processinfo.go Co-authored-by: Christos Kalkanis --- processmanager/processinfo.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 1553d21c4..7a4f5b8e5 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -524,8 +524,7 @@ func (pm *ProcessManager) processPIDExit(pid libpf.PID) { log.Debugf("Skip process exit handling for unknown PID %d", pid) return } - if pidExists || (pm.interpreterTracerEnabled && - len(pm.interpreters[pid]) > 0) { + if pm.interpreterTracerEnabled && len(pm.interpreters[pid]) > 0 { // ProcessPIDExit may be called multiple times in short succession // for the same PID, don't update exitKTime if we've previously recorded it. if _, pidExitProcessed := pm.exitEvents[pid]; !pidExitProcessed {