diff --git a/docs/features/process-start-time.md b/docs/features/process-start-time.md new file mode 100644 index 0000000000..662b471515 --- /dev/null +++ b/docs/features/process-start-time.md @@ -0,0 +1,205 @@ +# Process start time + +Every process in the process tree can carry a creation time. It exists to tell +apart two processes that share a process id — the kernel recycles ids, and +without a creation time a recycled id looks like the process that held it before. + +There are **two representations, and they are not interchangeable**: + +| Value | Where | Unit | Use for | +|---|---|---|---| +| `ProcessTreeManager.GetProcessBootTimeNs(pid)` | Creator side map | Nanoseconds since boot (`CLOCK_BOOTTIME`) | **Process identity.** The only value that may be compared. | +| `armotypes.Process.StartTime` | Tree node, and every alert's process tree | Wall-clock `time.Time` | **Display only.** Never compare it. | + +## Why two + +The identity value must be exact. The display value is derived through +`/proc/stat`'s `btime`, which has **whole-second resolution**, so it carries up +to a second of skew. Two processes created 40 ms apart can share a wall-clock +start time while their boot-relative values differ correctly. + +So: show `Process.StartTime` to a human, compare `GetProcessBootTimeNs` in code. +Anyone comparing the wall-clock value for identity is wrong by up to the btime +skew, and will be wrong intermittently rather than consistently. + +## Where the value comes from + +`/proc//stat` field 22 (`starttime`), the process's creation time in clock +ticks since boot. It is converted **once**, on the way in: + +```text +boot-relative nanoseconds = ticks × 10,000,000 (USER_HZ = 100) +``` + +Nothing downstream rescales it. A division at emission would compile, parse and +even join correctly *within* a single message, while silently breaking identity +across messages by seven orders of magnitude. If you find yourself scaling this +value on output, that is a bug. + +**The unit implies more precision than the value carries.** USER_HZ is 100, so +every value is an exact multiple of 10 ms. Do not treat it as a timestamp. + +### `(pid, startTimeNs)` is not guaranteed unique + +The 10 ms quantization means two processes created within the same tick share a +start time. For a **join key** built from `(pid, startTimeNs)` — which is what +the network-stream attribution work uses — that leaves a residual collision: a +process id recycled and reused inside the same 10 ms window produces two +incarnations with an identical tuple, and they are indistinguishable. + +This is narrow but real, and it is a limitation of the source rather than of +this code — `/proc` does not expose anything finer. Treat the tuple as a strong +discriminator, not a unique identifier, and do not build logic that assumes +two matching tuples must be the same process. + +**It is never derived from an event timestamp.** `convertExecEvent`, +`convertForkEvent` and `convertExitEvent` set `ProcessEvent.StartTimeNs` from the +event's wall-clock timestamp — epoch nanoseconds, a different clock domain, and +stamping the event instant rather than process creation. That value feeds +pending-exit sort ordering only and must never reach the identity map. An exec +event in particular stamps the exec, so a re-exec would change it. + +## When it gets populated + +Two paths, both reading the same kernel source: + +- **The periodic `/proc` scan** (every 30 s) — covers processes that predate the + agent, so coverage completes within one interval of startup. +- **On demand, when a fork or exec creates a tree node** — without this, every + process shorter than one scan interval would carry zero, and short-lived + processes are both the bulk of beacon-style connections and the drivers of + process-id churn. + +The on-demand read happens **once per node creation, never per event**. An exec +on a node that already has a value does not re-read: creation time cannot change +on exec. + +### Cost, and why the fork read is taken outside the tree lock + +A single `/proc//stat` open-read-parse measures **~7.5 µs** (Go benchmark, +Linux container, warm). Fork is the highest-volume event path on a node — +thousands per second on a busy one — and **every fork performs a read**, because +the recycled-pid guard below drops any stale value before the read decision is +made. + +At 1,000 forks/s that is ~7.5 ms of read time per second; at 3,000/s, ~22 ms/s. +Taken while holding `pt.mutex`, the tree-wide write lock, that would be hold +time blocking every reader of the process tree — and the tree is shared by every +alert type, not just the consumer this field was added for. + +So the fork path reads **before** acquiring the lock and stores the result under +it. Same number of reads, none of them holding the lock; fork contributes zero +added lock hold time. `TestHandleForkEvent_ReadsStartTimeWithoutHoldingTreeLock` +pins this, using `TryRLock` so a regression fails rather than deadlocks. + +Exec keeps its read inside the lock, where the skip-when-already-known check +makes it conditional and it only fires on first sight of a pid. + +The tradeoff: reading before the lock widens the window between the event and +the read. If the pid is recycled inside that window the value belongs to the new +incarnation — the same already-accepted race as before, just slightly wider, and +the reuse hardening is what detects it. + +### Zero means unknown + +A read fails if the process is already gone. The value stays zero rather than +being guessed. Callers must treat `0` as "unknown", never as "created at boot". + +Two populations keep a zero start time, both accepted: + +- Processes that died before their creation event was processed (sub-second + queue latency). +- In Kubernetes mode, pre-existing **host** (non-container) processes — the tree + refuses to create nodes for them at all. + +### Recycled process ids + +A fork's process id is newborn, so a surviving identity entry can only belong to +a process the kernel already recycled that id away from. The fork handler drops +the stale entry before reading, so the new process gets its own creation time. + +This matters because exits linger: `exitCleanup.cleanupDelay` defaults to five +minutes, so a dead process's tree node and its identity entry outlive it by that +long. + +The wipe is gated on the pid actually having a **pending exit**. An existing node +alone does not prove reuse — some other path may simply have created it first — +and wiping on that weaker signal would discard a good scan-recorded value +whenever the on-demand read then fails. + +> **This guard covers the identity map only.** It is **not** process-id-reuse +> hardening, and its benefit is temporary. +> +> The dead process's entry in `pendingExits` survives the fork. When the delayed +> cleanup fires — up to `cleanupDelay` after the *original* exit — it runs +> `exitByPid` on that pid and deletes what is by then the **live** successor's +> node, along with the start time just read for it. The pid reverts to reading +> as unknown. +> +> During the same window the node also still carries the dead process's `comm`, +> `cmdline` and `path`, so an alert can name the wrong command. +> +> Both are the pre-existing pid-reuse behaviour, tracked separately. The fix is +> to retire the predecessor when a fork reuses a pid with a pending exit — +> reparenting its children rather than merely dropping the pending entry, which +> would leave them to be inherited by the successor. + +The entry is deleted at the same point the tree node is deleted. Note that per +the above, the node deleted may by then belong to a different process than the +one whose exit scheduled the deletion. + +## Reading it + +```go +// Identity — the only value safe to compare. +ns := processTreeManager.GetProcessBootTimeNs(pid) +if ns == 0 { + // Unknown: not yet scanned, died before its creation event, or a host + // process in Kubernetes mode. Fall back to process-id-only handling. +} + +// Display — carried on every node of every alert's process tree. +node.StartTime // wall clock, up to 1s of skew, never compare +``` + +`Process.StartTime` reaches consumers only because three separate copy functions +were taught to carry it. Each builds a node from an **explicit field list** and +silently drops anything absent from it: + +- `containerprocesstree.buildBranchToShim` — every alert branch and every + network-stream tree +- `utils.CopyProcess` — the alert bulk manager's merged tree +- `utils.EnrichProcess` — the bulk manager's merge of overlapping chains + +Each has a dedicated test asserting the field survives. **If you add a field to +`armotypes.Process` and expect it downstream, you must update all three** — +otherwise it is populated at the source and silently empty at the consumer. + +One builder deliberately still strips it: `utils.CreateProcessTree` +(`pkg/utils/process.go`), used by the malware manager, so malware alerts carry a +zero start time. Tracked separately. + +## A note on the wire + +`armotypes.Process.StartTime` is a `time.Time` tagged `omitempty`, and Go's +encoder ignores `omitempty` on structs. The field has therefore always been +serialised — as `"startTime":"0001-01-01T00:00:00Z"` — so populating it changes +an existing value rather than adding a new one. Nothing branches on it and there +is no hash or equality over `Process`, but consumers do see the change. + +## Testing + +This package cannot be built or tested on macOS: `inspektor-gadget/pkg/utils/host` +excludes darwin and everything under `pkg/processtree` imports it transitively. + +```bash +docker run --rm -v "$PWD":/src -v "$(go env GOMODCACHE)":/go/pkg/mod -w /src \ + -e GOFLAGS=-mod=mod golang:1.25 go test ./pkg/processtree/... ./pkg/utils/... +``` + +The tick conversion is defined once per package (feeder and creator). Both are +pinned in tests against an independently read field 22 times a literal `10^7`, +so if the two ever drift, a test fails deterministically. Weaker forms of that +check are not enough: `ns % 10^7 == 0` only catches a 10× error when +`ticks % 10 != 0`, and comparing the wall-clock value against "now" loses +sensitivity on a freshly booted machine. diff --git a/pkg/containerwatcher/v2/tracers/procfs.go b/pkg/containerwatcher/v2/tracers/procfs.go index eefaca660b..2c91a5f028 100644 --- a/pkg/containerwatcher/v2/tracers/procfs.go +++ b/pkg/containerwatcher/v2/tracers/procfs.go @@ -161,6 +161,7 @@ func (pt *ProcfsTracer) handleProcfsEvent(event conversion.ProcessEvent) { Cwd: event.Cwd, Path: event.Path, StartTimeNs: event.StartTimeNs, + StartTimeWall: event.StartTimeWall, ContainerID: event.ContainerID, ContainerMntNs: event.ContainerMntNs, ContainerNetNs: event.ContainerNetNs, diff --git a/pkg/ebpf/events/procfs.go b/pkg/ebpf/events/procfs.go index 60b1f37505..7856969785 100644 --- a/pkg/ebpf/events/procfs.go +++ b/pkg/ebpf/events/procfs.go @@ -1,29 +1,35 @@ package events import ( + "time" + "github.com/inspektor-gadget/inspektor-gadget/pkg/types" "github.com/kubescape/node-agent/pkg/utils" ) // ProcfsEvent represents a procfs event that can be processed by the ordered event queue type ProcfsEvent struct { - Type types.EventType `json:"type"` - Timestamp types.Time `json:"timestamp"` - PID uint32 `json:"pid"` - PPID uint32 `json:"ppid"` - Comm string `json:"comm"` - Pcomm string `json:"pcomm"` - Cmdline string `json:"cmdline"` - Uid *uint32 `json:"uid"` - Gid *uint32 `json:"gid"` - Cwd string `json:"cwd"` - Path string `json:"path"` - StartTimeNs uint64 `json:"start_time_ns"` - ContainerID string `json:"container_id"` - ContainerMntNs uint64 `json:"container_mnt_ns"` - ContainerNetNs uint64 `json:"container_net_ns"` - HostPID int `json:"host_pid"` - HostPPID int `json:"host_ppid"` + Type types.EventType `json:"type"` + Timestamp types.Time `json:"timestamp"` + PID uint32 `json:"pid"` + PPID uint32 `json:"ppid"` + Comm string `json:"comm"` + Pcomm string `json:"pcomm"` + Cmdline string `json:"cmdline"` + Uid *uint32 `json:"uid"` + Gid *uint32 `json:"gid"` + Cwd string `json:"cwd"` + Path string `json:"path"` + StartTimeNs uint64 `json:"start_time_ns"` + // StartTimeWall is the display-only wall-clock creation time derived by the + // procfs feeder from btime + StartTimeNs. Never compare it for process + // identity — StartTimeNs (boot-relative) is the sole identity source. + StartTimeWall time.Time `json:"start_time_wall"` + ContainerID string `json:"container_id"` + ContainerMntNs uint64 `json:"container_mnt_ns"` + ContainerNetNs uint64 `json:"container_net_ns"` + HostPID int `json:"host_pid"` + HostPPID int `json:"host_ppid"` } var _ utils.K8sEvent = (*ProcfsEvent)(nil) diff --git a/pkg/processtree/container/container_processtree.go b/pkg/processtree/container/container_processtree.go index 67cf1ab8b5..afd55db8cd 100644 --- a/pkg/processtree/container/container_processtree.go +++ b/pkg/processtree/container/container_processtree.go @@ -240,6 +240,7 @@ func (c *containerProcessTreeImpl) buildBranchToShim(targetNode *armotypes.Proce Gid: node.Gid, Cwd: node.Cwd, Path: node.Path, + StartTime: node.StartTime, ChildrenMap: make(map[armotypes.CommPID]*armotypes.Process), } } diff --git a/pkg/processtree/container/container_processtree_test.go b/pkg/processtree/container/container_processtree_test.go index 72dd99f006..d0273ed1a7 100644 --- a/pkg/processtree/container/container_processtree_test.go +++ b/pkg/processtree/container/container_processtree_test.go @@ -2,6 +2,7 @@ package containerprocesstree import ( "testing" + "time" "github.com/armosec/armoapi-go/armotypes" "github.com/goradd/maps" @@ -821,3 +822,51 @@ func TestContainerProcessTreeImpl_IsProcessUnderAnyContainerSubtree_OutsideProce assert.False(t, cpt.IsProcessUnderAnyContainerSubtree(200, createSafeMapFromData(fullTree))) // outside process assert.False(t, cpt.IsProcessUnderAnyContainerSubtree(1, createSafeMapFromData(fullTree))) // init } + +// TestContainerProcessTreeImpl_GetPidBranch_CarriesStartTime pins the third of +// the three copy sites that strip any field they do not explicitly list. +// buildBranchToShim feeds GetContainerProcessTree, so every alert branch and +// every network-stream tree goes through it: if the node literal drops +// StartTime, the value is populated on the tree and still never reaches a +// consumer. +func TestContainerProcessTreeImpl_GetPidBranch_CarriesStartTime(t *testing.T) { + cpt := NewContainerProcessTree().(*containerProcessTreeImpl) + + containerID := "test-container-123" + shimPID := uint32(50) + cpt.containerIdToShimPid[containerID] = shimPID + + nginxStart := time.Date(2026, 7, 29, 10, 0, 0, 0, time.UTC) + workerStart := time.Date(2026, 7, 29, 10, 5, 0, 0, time.UTC) + + nginxWorker := &armotypes.Process{ + PID: 101, + PPID: 100, + Comm: "nginx-worker", + StartTime: workerStart, + ChildrenMap: map[armotypes.CommPID]*armotypes.Process{}, + } + nginxProcess := &armotypes.Process{ + PID: 100, + PPID: 50, // parent is shim + Comm: "nginx", + StartTime: nginxStart, + ChildrenMap: map[armotypes.CommPID]*armotypes.Process{ + {Comm: "nginx-worker", PID: 101}: nginxWorker, + }, + } + fullTree := map[uint32]*armotypes.Process{ + 1: {PID: 1, PPID: 0, Comm: "init"}, + 50: {PID: 50, PPID: 1, Comm: "containerd-shim", ChildrenMap: map[armotypes.CommPID]*armotypes.Process{{Comm: "nginx", PID: 100}: nginxProcess}}, + 100: nginxProcess, + 101: nginxWorker, + } + + result, err := cpt.GetPidBranch(containerID, 101, createSafeMapFromData(fullTree)) + assert.NoError(t, err) + assert.Equal(t, nginxStart, result.StartTime, "branch root must carry the source node's StartTime") + + child, ok := result.ChildrenMap[armotypes.CommPID{Comm: "nginx-worker", PID: 101}] + assert.True(t, ok) + assert.Equal(t, workerStart, child.StartTime, "branch leaf must carry the source node's StartTime") +} diff --git a/pkg/processtree/conversion/convert.go b/pkg/processtree/conversion/convert.go index ff54c6d5c9..4be31dd6fb 100644 --- a/pkg/processtree/conversion/convert.go +++ b/pkg/processtree/conversion/convert.go @@ -131,9 +131,14 @@ func convertProcfsEvent(procfsEvent *events.ProcfsEvent) ProcessEvent { Cwd: procfsEvent.Cwd, Path: procfsEvent.Path, StartTimeNs: procfsEvent.StartTimeNs, - ContainerID: procfsEvent.ContainerID, - HostPID: procfsEvent.HostPID, - HostPPID: procfsEvent.HostPPID, + // Only ProcfsEvent carries a real start time. The exec/fork/exit + // converters above set StartTimeNs from the event's wall-clock timestamp + // — a different clock domain (epoch ns, not boot ns) with different + // semantics — and deliberately leave StartTimeWall zero. + StartTimeWall: procfsEvent.StartTimeWall, + ContainerID: procfsEvent.ContainerID, + HostPID: procfsEvent.HostPID, + HostPPID: procfsEvent.HostPPID, } return event diff --git a/pkg/processtree/conversion/convert_test.go b/pkg/processtree/conversion/convert_test.go new file mode 100644 index 0000000000..6f4b4d1236 --- /dev/null +++ b/pkg/processtree/conversion/convert_test.go @@ -0,0 +1,26 @@ +package conversion + +import ( + "testing" + "time" + + "github.com/inspektor-gadget/inspektor-gadget/pkg/types" + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertProcfsEvent_CarriesStartTime(t *testing.T) { + wall := time.Date(2026, 7, 29, 10, 0, 0, 0, time.UTC) + pe := &events.ProcfsEvent{ + Type: types.NORMAL, Timestamp: types.Time(time.Now().UnixNano()), + PID: 42, PPID: 1, Comm: "nginx", + StartTimeNs: 123_450_000_000, // 12345 ticks + StartTimeWall: wall, + } + got, err := ConvertEvent(utils.ProcfsEventType, pe) + require.NoError(t, err) + assert.Equal(t, uint64(123_450_000_000), got.StartTimeNs) + assert.Equal(t, wall, got.StartTimeWall) +} diff --git a/pkg/processtree/conversion/types.go b/pkg/processtree/conversion/types.go index ce156d2d6b..cb05e2fae3 100644 --- a/pkg/processtree/conversion/types.go +++ b/pkg/processtree/conversion/types.go @@ -28,6 +28,12 @@ type ProcessEvent struct { Cwd string Path string StartTimeNs uint64 // Process start time in nanoseconds for unique identification + // StartTimeWall is the process creation time as wall-clock time, derived by + // the procfs feeder from /proc/stat btime + StartTimeNs. DISPLAY ONLY: btime + // has whole-second resolution, so this value carries up to 1s of skew and + // must never be compared for process identity — StartTimeNs is the sole + // identity source. Zero for every event type except ProcfsEvent. + StartTimeWall time.Time // Container context ContainerID string diff --git a/pkg/processtree/creator/exit_manager.go b/pkg/processtree/creator/exit_manager.go index 69a29e5ff0..a26c78c6d3 100644 --- a/pkg/processtree/creator/exit_manager.go +++ b/pkg/processtree/creator/exit_manager.go @@ -128,6 +128,7 @@ func (pt *processTreeCreatorImpl) exitByPid(pid uint32) { proc, ok := pt.processMap.Load(pid) if !ok { delete(pt.pendingExits, pid) + delete(pt.pidStartTimeNs, pid) return } @@ -181,5 +182,7 @@ func (pt *processTreeCreatorImpl) exitByPid(pid uint32) { } pt.processMap.Delete(pid) + // The process identity dies with the node it identifies. + delete(pt.pidStartTimeNs, pid) delete(pt.pendingExits, pid) } diff --git a/pkg/processtree/creator/processtree_creator.go b/pkg/processtree/creator/processtree_creator.go index 2f79a3a516..4d508b2451 100644 --- a/pkg/processtree/creator/processtree_creator.go +++ b/pkg/processtree/creator/processtree_creator.go @@ -3,6 +3,7 @@ package processtreecreator import ( "fmt" "sync" + "time" "github.com/armosec/armoapi-go/armotypes" "github.com/goradd/maps" @@ -24,6 +25,21 @@ type processTreeCreatorImpl struct { // Exit manager fields pendingExits map[uint32]*pendingExit // PID -> pending exit exitCleanupStopChan chan struct{} + + // pidStartTimeNs maps a live pid to its creation time in nanoseconds since + // boot, sourced EXCLUSIVELY from /proc//stat field 22 (never from + // fork/exec/exit event timestamps, which are epoch wall-clock — a different + // clock domain). This side map is the SOLE process-identity time source; the + // wall-clock Process.StartTime stamped on tree nodes is display-only and + // inherits btime's whole-second skew. Guarded by pt.mutex. Entries are + // deleted in exitByPid together with the tree node. + pidStartTimeNs map[uint32]uint64 + + // readStartTime fetches a pid's boot-relative start time and display-only + // wall time from /proc on demand, so a node created by a fork or exec event + // does not wait up to a full scan interval for its identity. Returns zeros + // when the process is already gone. Injectable for tests. + readStartTime func(pid uint32) (uint64, time.Time) } func NewProcessTreeCreator(containerTree containerprocesstree.ContainerProcessTree, config config.Config) ProcessTreeCreator { @@ -39,6 +55,8 @@ func NewProcessTreeCreator(containerTree containerprocesstree.ContainerProcessTr reparentingStrategies: reparentingLogic, containerTree: containerTree, pendingExits: make(map[uint32]*pendingExit), + pidStartTimeNs: make(map[uint32]uint64), + readStartTime: newProcfsStartTimeReader(), config: config, } @@ -98,6 +116,16 @@ func (pt *processTreeCreatorImpl) GetProcessNode(pid int) (*armotypes.Process, e return pt.shallowCopyProcess(proc), nil } +// GetProcessBootTimeNs returns the pid's creation time as nanoseconds since boot +// (CLOCK_BOOTTIME), or 0 when unknown — the process has not been seen by the +// /proc scan or an on-demand read, or it has already exited. Procfs-sourced +// only; see pidStartTimeNs for the identity contract. +func (pt *processTreeCreatorImpl) GetProcessBootTimeNs(pid uint32) uint64 { + pt.mutex.RLock() + defer pt.mutex.RUnlock() + return pt.pidStartTimeNs[pid] +} + // GetPidBranch performs container branch operation (no longer needs to be atomic) func (pt *processTreeCreatorImpl) GetPidBranch(containerTree interface{}, containerID string, targetPID uint32) (armotypes.Process, error) { pt.mutex.RLock() @@ -139,14 +167,83 @@ func (pt *processTreeCreatorImpl) UpdatePPID(proc *armotypes.Process, event conv } } +// ensureStartTime gives a fork/exec-created node its process identity without +// waiting up to a full scan interval for the periodic /proc sweep — short-lived +// processes are the population most exposed to pid reuse, and they are exactly +// the ones the scan misses. +// +// One read per node creation, never per event: an exec on a node whose entry +// already exists must not re-read, because creation time cannot change on exec. +// +// event.StartTimeNs is deliberately ignored. For fork and exec events it is the +// event's wall-clock timestamp — epoch nanoseconds, not boot-relative, and +// stamping the event rather than process creation. Only the procfs read may +// populate the side map. A failed read leaves zero, never a guess. +// +// Must be called with pt.mutex held. +func (pt *processTreeCreatorImpl) ensureStartTime(proc *armotypes.Process, pid uint32) { + if _, known := pt.pidStartTimeNs[pid]; known { + return + } + ns, wall := pt.readStartTime(pid) + pt.applyStartTime(proc, pid, ns, wall) +} + +// applyStartTime stores an already-read start time. Split out from +// ensureStartTime so the fork path can perform its /proc read before taking +// pt.mutex: fork is the highest-volume event path and always reads (the +// recycled-pid guard drops any stale entry), so reading under the tree-wide +// write lock would serialise a file read into the hot path for every alert +// type. Exec keeps the read inside ensureStartTime, where the +// skip-when-already-known check makes it conditional. +// +// A zero ns means the read failed — leave the pre-existing zero, never guess. +// +// Must be called with pt.mutex held. +func (pt *processTreeCreatorImpl) applyStartTime(proc *armotypes.Process, pid uint32, ns uint64, wall time.Time) { + if ns == 0 { + return + } + pt.pidStartTimeNs[pid] = ns + if !wall.IsZero() { + proc.StartTime = wall + } +} + // handleForkEvent handles fork events - only fills properties if they are empty or don't exist func (pt *processTreeCreatorImpl) handleForkEvent(event conversion.ProcessEvent) { + // Read the start time BEFORE taking the tree lock. A fork always needs it — + // the pid is newborn, and the guard below drops any stale value — so this is + // the same number of reads as doing it under the lock, with none of them + // holding it. Widening the gap between event and read only widens an + // already-accepted race: if the pid is recycled in between, the value read + // belongs to the current incarnation, which the reuse hardening detects. + startTimeNs, startTimeWall := pt.readStartTime(event.PID) + pt.mutex.Lock() defer pt.mutex.Unlock() proc, ok := pt.processMap.Load(event.PID) if !ok { proc = pt.getOrCreateProcess(event.PID) + } else if _, exited := pt.pendingExits[event.PID]; exited { + // The previous holder of this pid exited but its node is still around — + // exits linger for exitCleanup.cleanupDelay, 5 minutes by default — so a + // fork on it means the kernel recycled the pid. Drop BOTH halves of the + // stale start time, the identity value and the display value, so they + // cannot disagree, and let applyStartTime restamp them below. + // + // Gated on a pending exit rather than on the node merely existing: an + // existing node alone only means some other path created it first, and + // wiping on that weaker signal would discard a good scan-recorded value + // whenever the on-demand read then fails. + // + // Scoped to the start time. This is NOT pid-reuse hardening: the node + // still carries the dead process's comm, cmdline and path, and the dead + // process's pendingExits entry still schedules a delayed exitByPid that + // will remove this live successor's node. Both belong to SUB-7846. + delete(pt.pidStartTimeNs, event.PID) + proc.StartTime = time.Time{} } pt.UpdatePPID(proc, event) @@ -173,6 +270,8 @@ func (pt *processTreeCreatorImpl) handleForkEvent(event conversion.ProcessEvent) proc.Path = event.Path } + pt.applyStartTime(proc, event.PID, startTimeNs, startTimeWall) + if proc.ChildrenMap == nil { proc.ChildrenMap = make(map[armotypes.CommPID]*armotypes.Process) } @@ -217,6 +316,16 @@ func (pt *processTreeCreatorImpl) handleProcfsEvent(event conversion.ProcessEven proc.Path = event.Path } + // Start time from /proc//stat field 22. The boot-relative value is the + // identity source and lives only in the side map; the wall-clock value is + // stamped on the node for display and must never be compared for identity. + if event.StartTimeNs != 0 { + pt.pidStartTimeNs[event.PID] = event.StartTimeNs + if !event.StartTimeWall.IsZero() { + proc.StartTime = event.StartTimeWall + } + } + if proc.ChildrenMap == nil { proc.ChildrenMap = make(map[armotypes.CommPID]*armotypes.Process) } @@ -264,6 +373,9 @@ func (pt *processTreeCreatorImpl) handleExecEvent(event conversion.ProcessEvent) if event.Path != "" { proc.Path = event.Path } + + pt.ensureStartTime(proc, event.PID) + if proc.ChildrenMap == nil { proc.ChildrenMap = make(map[armotypes.CommPID]*armotypes.Process) } diff --git a/pkg/processtree/creator/processtree_creator_interface.go b/pkg/processtree/creator/processtree_creator_interface.go index 01a7f2328d..1a00c34fd3 100644 --- a/pkg/processtree/creator/processtree_creator_interface.go +++ b/pkg/processtree/creator/processtree_creator_interface.go @@ -15,6 +15,10 @@ type ProcessTreeCreator interface { GetProcessMap() *maps.SafeMap[uint32, *armotypes.Process] // Optionally: Query for a process node by PID GetProcessNode(pid int) (*armotypes.Process, error) + // GetProcessBootTimeNs returns the process's creation time as nanoseconds + // since boot (CLOCK_BOOTTIME), sourced exclusively from /proc//stat + // field 22. Returns 0 when unknown (process not yet seen, or already exited). + GetProcessBootTimeNs(pid uint32) uint64 // Start the process tree creator and begin background tasks Start() // Stop the process tree creator and cleanup resources diff --git a/pkg/processtree/creator/starttime_reader.go b/pkg/processtree/creator/starttime_reader.go new file mode 100644 index 0000000000..5f0b6daab0 --- /dev/null +++ b/pkg/processtree/creator/starttime_reader.go @@ -0,0 +1,76 @@ +package processtreecreator + +import ( + "sync" + "time" + + "github.com/kubescape/go-logger" + "github.com/kubescape/go-logger/helpers" + "github.com/prometheus/procfs" +) + +// nsPerTick converts /proc//stat field 22 (clock ticks since boot) to +// boot-relative nanoseconds. USER_HZ is 100, so this is exact: 1e9 / 100. +// The procfs feeder defines the same constant for the periodic scan; both are +// package-private and must stay in agreement. +const nsPerTick = uint64(10_000_000) + +// newProcfsStartTimeReader returns the on-demand equivalent of the periodic +// scan's start-time read: /proc//stat field 22 (creation time in clock +// ticks since boot), converted to boot-relative nanoseconds plus the derived +// display-only wall time. +// +// This is NOT the rejected "derive the start time from an exec event" option. +// That rejection was of event TIMESTAMPS — an exec event stamps the exec +// instant, not process creation, so the value changes when a process re-execs. +// This reads the same kernel source as the periodic scan, at the same +// semantics, just fetched when the node is created instead of waiting up to a +// full scan interval. Without it every process shorter than one scan interval +// carries a zero start time, and short-lived processes are exactly the +// population most exposed to pid reuse. +// +// Returns zeros when the process is already gone (it died before its creation +// event was processed) — the caller keeps the pre-existing zero-value +// behaviour rather than guessing. +func newProcfsStartTimeReader() func(pid uint32) (uint64, time.Time) { + var ( + once sync.Once + fs procfs.FS + fsReady bool + bootTime time.Time + ) + return func(pid uint32) (uint64, time.Time) { + // Resolve /proc and btime once, not per read: procfs.NewProc would + // re-stat the mount point on every call. + once.Do(func() { + f, err := procfs.NewDefaultFS() + if err != nil { + logger.L().Warning("processtree: cannot open /proc, on-demand start times disabled", helpers.Error(err)) + return + } + fs, fsReady = f, true + if s, err := f.Stat(); err == nil { + bootTime = time.Unix(int64(s.BootTime), 0) + } else { + // Boot-relative identity is unaffected; only the display value is lost. + logger.L().Warning("processtree: cannot read /proc/stat btime, wall-clock start times disabled", helpers.Error(err)) + } + }) + if !fsReady { + return 0, time.Time{} + } + proc, err := fs.Proc(int(pid)) + if err != nil { + return 0, time.Time{} + } + stat, err := proc.Stat() + if err != nil || stat.Starttime == 0 { + return 0, time.Time{} + } + ns := stat.Starttime * nsPerTick + if bootTime.IsZero() { + return ns, time.Time{} + } + return ns, bootTime.Add(time.Duration(ns)) + } +} diff --git a/pkg/processtree/creator/starttime_test.go b/pkg/processtree/creator/starttime_test.go new file mode 100644 index 0000000000..0d02c661db --- /dev/null +++ b/pkg/processtree/creator/starttime_test.go @@ -0,0 +1,308 @@ +package processtreecreator + +import ( + "os" + "testing" + "time" + + "github.com/prometheus/procfs" + + "github.com/kubescape/node-agent/pkg/config" + processtreecreatorconfig "github.com/kubescape/node-agent/pkg/processtree/config" + "github.com/kubescape/node-agent/pkg/processtree/conversion" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHandleProcfsEvent_RecordsStartTime(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}) + wall := time.Date(2026, 7, 29, 10, 0, 0, 0, time.UTC) + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ProcfsEvent, Timestamp: time.Now(), + PID: 100, PPID: 1, Comm: "nginx", + StartTimeNs: 555_550_000_000, StartTimeWall: wall, + }) + + // Boot-ns identity: exposed via the accessor, sourced from the side map. + assert.Equal(t, uint64(555_550_000_000), creator.GetProcessBootTimeNs(100)) + assert.Zero(t, creator.GetProcessBootTimeNs(999), "unknown pid reads as 0") + + // Wall-clock display value on the node. + node, err := creator.GetProcessNode(100) + require.NoError(t, err) + require.NotNil(t, node) + assert.Equal(t, wall, node.StartTime) +} + +func TestExitByPid_DeletesStartTimeEntry(t *testing.T) { + cfg := config.Config{} + cfg.ExitCleanup = processtreecreatorconfig.ExitCleanupConfig{ + MaxPendingExits: 10, CleanupInterval: time.Hour, CleanupDelay: 0, + } + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, cfg).(*processTreeCreatorImpl) + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ProcfsEvent, PID: 100, PPID: 1, Comm: "nginx", + StartTimeNs: 1_000_000_000, + }) + require.Equal(t, uint64(1_000_000_000), creator.GetProcessBootTimeNs(100), + "precondition: the side map must hold the entry before the exit") + + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ExitEvent, PID: 100, Comm: "exit", Timestamp: time.Now(), + }) + creator.mutex.Lock() + creator.exitByPid(100) + creator.mutex.Unlock() + + assert.Zero(t, creator.GetProcessBootTimeNs(100), "side-map entry must die with the node") +} + +func TestHandleForkEvent_ReadsStartTimeOnDemand(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}).(*processTreeCreatorImpl) + wall := time.Date(2026, 7, 29, 12, 0, 0, 0, time.UTC) + reads := 0 + creator.readStartTime = func(pid uint32) (uint64, time.Time) { + reads++ + require.Equal(t, uint32(100), pid) + return 777_770_000_000, wall + } + + creator.FeedEvent(conversion.ProcessEvent{Type: conversion.ForkEvent, PID: 100, PPID: 1, Comm: "curl"}) + assert.Equal(t, uint64(777_770_000_000), creator.GetProcessBootTimeNs(100), + "a fork-created node must not wait up to a scan interval for its identity") + node, err := creator.GetProcessNode(100) + require.NoError(t, err) + assert.Equal(t, wall, node.StartTime) + assert.Equal(t, 1, reads) + + // Exec on the same node: creation time cannot change on exec — no re-read. + creator.FeedEvent(conversion.ProcessEvent{Type: conversion.ExecEvent, PID: 100, PPID: 1, Comm: "curl", Cmdline: "curl -s"}) + assert.Equal(t, 1, reads, "one read per node creation, never per event") +} + +func TestHandleExecEvent_ReadsStartTimeOnDemand(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}).(*processTreeCreatorImpl) + wall := time.Date(2026, 7, 29, 13, 0, 0, 0, time.UTC) + creator.readStartTime = func(pid uint32) (uint64, time.Time) { return 888_880_000_000, wall } + + // An exec with no preceding fork event still creates the node, so it must + // also acquire an identity rather than waiting for the next scan. + creator.FeedEvent(conversion.ProcessEvent{Type: conversion.ExecEvent, PID: 200, PPID: 1, Comm: "sh"}) + assert.Equal(t, uint64(888_880_000_000), creator.GetProcessBootTimeNs(200)) + node, err := creator.GetProcessNode(200) + require.NoError(t, err) + assert.Equal(t, wall, node.StartTime) +} + +func TestHandleForkEvent_ReadFailureLeavesZero(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}).(*processTreeCreatorImpl) + creator.readStartTime = func(pid uint32) (uint64, time.Time) { return 0, time.Time{} } // process already gone + creator.FeedEvent(conversion.ProcessEvent{Type: conversion.ForkEvent, PID: 100, PPID: 1, Comm: "flash"}) + assert.Zero(t, creator.GetProcessBootTimeNs(100), "failure degrades to the pre-existing zero, never a guess") +} + +// TestHandleForkEvent_IgnoresEventStartTimeNs pins the clock-domain boundary. +// convertForkEvent sets ProcessEvent.StartTimeNs from the event's wall-clock +// timestamp — epoch nanoseconds, not boot-relative, and stamping the fork rather +// than process creation. Letting that value into the side map would make identity +// comparisons wrong by decades while still compiling and still joining correctly +// within a single message. Only the procfs read may populate the side map. +func TestHandleForkEvent_IgnoresEventStartTimeNs(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}).(*processTreeCreatorImpl) + creator.readStartTime = func(pid uint32) (uint64, time.Time) { return 777_770_000_000, time.Time{} } + + const epochNs = uint64(1_753_876_800_000_000_000) // what convertForkEvent would supply + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ForkEvent, PID: 100, PPID: 1, Comm: "curl", StartTimeNs: epochNs, + }) + + assert.Equal(t, uint64(777_770_000_000), creator.GetProcessBootTimeNs(100), + "the side map must hold the procfs boot-relative read, not the event's epoch timestamp") +} + +// TestHandleForkEvent_IgnoresEventStartTimeNs_OnReadFailure is the same boundary +// on the failure path: a failed read must leave zero, never fall back to the +// event's epoch timestamp. +func TestHandleForkEvent_IgnoresEventStartTimeNs_OnReadFailure(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}).(*processTreeCreatorImpl) + creator.readStartTime = func(pid uint32) (uint64, time.Time) { return 0, time.Time{} } + + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ForkEvent, PID: 100, PPID: 1, Comm: "curl", + StartTimeNs: 1_753_876_800_000_000_000, + }) + + assert.Zero(t, creator.GetProcessBootTimeNs(100), + "a failed read must degrade to zero, never to the event's epoch timestamp") +} + +// TestExitByPid_DeletesStartTimeEntry_NodeAlreadyGone covers exitByPid's early +// return: the node is absent from processMap, but a stale side-map entry must +// still be reclaimed rather than leaking for the lifetime of the agent. +func TestExitByPid_DeletesStartTimeEntry_NodeAlreadyGone(t *testing.T) { + cfg := config.Config{} + cfg.ExitCleanup = processtreecreatorconfig.ExitCleanupConfig{ + MaxPendingExits: 10, CleanupInterval: time.Hour, CleanupDelay: 0, + } + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, cfg).(*processTreeCreatorImpl) + + creator.mutex.Lock() + creator.pidStartTimeNs[404] = 2_000_000_000 // entry with no corresponding node + creator.exitByPid(404) + creator.mutex.Unlock() + + assert.Zero(t, creator.GetProcessBootTimeNs(404), + "the early-return branch must reclaim the side-map entry too") +} + +// TestProcfsStartTimeReader_ReadsRealProcess covers newProcfsStartTimeReader +// itself. Every other test on the on-demand path injects a fake reader, so +// without this the real /proc parse and its tick->nanosecond conversion have no +// coverage at all: this package's nsPerTick could drift from the feeder's and +// the same process would get two identities an order of magnitude apart, each +// internally consistent. +// +// The conversion is pinned against an independently read field 22 times the +// contract's literal 10^7, so a drifted constant fails deterministically rather +// than only on machines whose uptime happens to make the error visible. +func TestProcfsStartTimeReader_ReadsRealProcess(t *testing.T) { + read := newProcfsStartTimeReader() + + ns, wall := read(uint32(os.Getpid())) + require.NotZero(t, ns, "the test's own process must be readable from /proc") + + p, err := procfs.NewProc(os.Getpid()) + require.NoError(t, err) + stat, err := p.Stat() + require.NoError(t, err) + assert.Equal(t, stat.Starttime*10_000_000, ns, + "conversion must be exactly field 22 ticks * 10^7 ns (USER_HZ=100)") + + assert.False(t, wall.IsZero(), "btime should be readable, so the display value should be set") + assert.WithinDuration(t, time.Now(), wall, 5*time.Minute, + "btime + boot-relative offset must reconstruct this process's actual start time") + + // A pid that cannot exist degrades to zeros rather than guessing. + nsGone, wallGone := read(uint32(1 << 22)) + assert.Zero(t, nsGone, "an unreadable process must yield zero, never a guess") + assert.True(t, wallGone.IsZero()) +} + +// TestHandleForkEvent_RecycledPidDoesNotInheritDeadProcessStartTime guards the +// case the start time exists to catch. A fork's pid is newborn by definition, so +// a surviving side-map entry can only mean the kernel recycled the pid. +// +// It is reachable with shipped defaults: exitCleanup.cleanupDelay is 5 minutes, +// so an exited process's tree node AND its side-map entry outlive it by that +// long. Without the guard the recycled pid reports the DEAD process's start +// time, and if the new process lives less than one 30s scan interval — the +// short-lived population the on-demand read was added for — the periodic scan +// never corrects it. That is worse than the zero it replaces: a consumer joining +// on (pid, startTime) concludes the two processes are one. +// +// Scoped to this package's side map only. This is NOT pid-reuse hardening: the +// shared tree node still carries the dead process's comm, cmdline and path. +func TestHandleForkEvent_RecycledPidDoesNotInheritDeadProcessStartTime(t *testing.T) { + cfg := config.Config{} + cfg.ExitCleanup = processtreecreatorconfig.ExitCleanupConfig{ + MaxPendingExits: 1000, + CleanupInterval: 30 * time.Second, + CleanupDelay: 5 * time.Minute, // shipped default + } + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, cfg).(*processTreeCreatorImpl) + + const aStart = uint64(1_000_000_000) // process A, boot+1s + const bStart = uint64(9_000_000_000) // process B, boot+9s + aWall := time.Date(2026, 7, 29, 10, 0, 0, 0, time.UTC) + bWall := time.Date(2026, 7, 29, 10, 30, 0, 0, time.UTC) + creator.readStartTime = func(pid uint32) (uint64, time.Time) { return bStart, bWall } + + // Process A on pid 4242, seen by the periodic scan. + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ProcfsEvent, PID: 4242, PPID: 1, Comm: "victim", + StartTimeNs: aStart, StartTimeWall: aWall, + }) + require.Equal(t, aStart, creator.GetProcessBootTimeNs(4242)) + + // A exits. Cleanup is delayed, so the node and the side-map entry survive. + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ExitEvent, PID: 4242, Comm: "exit", Timestamp: time.Now(), + }) + + // The kernel recycles 4242 for process B; B's fork event arrives. + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ForkEvent, PID: 4242, PPID: 1, Comm: "beacon", + }) + + assert.Equal(t, bStart, creator.GetProcessBootTimeNs(4242), + "a newborn pid must get its OWN start time, not the dead process's") + + // The node is reused, so its display value must be restamped too. If only the + // side map is cleared, the identity value and the value shown to a human + // disagree for exactly the case this guard exists to handle. + node, err := creator.GetProcessNode(4242) + require.NoError(t, err) + require.NotNil(t, node) + assert.Equal(t, bWall, node.StartTime, + "the reused node must not keep displaying the dead process's start time") +} + +// TestHandleForkEvent_ReadsStartTimeWithoutHoldingTreeLock pins that the fork +// path's /proc read happens OUTSIDE pt.mutex. +// +// pt.mutex is the tree-wide write lock that every alert type contends on, and +// fork is the highest-volume event path on a node — thousands per second on a +// busy one. Since the recycled-pid guard drops any stale entry, every fork now +// performs a read, so holding the lock across it would serialise a file read +// into the hot path for the whole process tree. +// +// TryRLock rather than RLock: if the write lock were held, RLock from this same +// goroutine would deadlock and the test would hang instead of failing. +func TestHandleForkEvent_ReadsStartTimeWithoutHoldingTreeLock(t *testing.T) { + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, config.Config{}).(*processTreeCreatorImpl) + + readWasLockFree := false + creator.readStartTime = func(pid uint32) (uint64, time.Time) { + if creator.mutex.TryRLock() { + readWasLockFree = true + creator.mutex.RUnlock() + } + return 555_550_000_000, time.Time{} + } + + creator.FeedEvent(conversion.ProcessEvent{Type: conversion.ForkEvent, PID: 100, PPID: 1, Comm: "curl"}) + + assert.True(t, readWasLockFree, + "the fork path's /proc read must not hold the tree-wide write lock") + assert.Equal(t, uint64(555_550_000_000), creator.GetProcessBootTimeNs(100), + "and the value must still be stored") +} + +// A fork on an existing node does not always mean pid reuse — some other path +// may simply have created the node first. Wiping unconditionally destroys a +// good scan-recorded value whenever the on-demand read then fails, so the wipe +// is gated on there actually being a pending exit for the pid. +func TestHandleForkEvent_DoesNotWipeKnownStartTimeWithoutAPendingExit(t *testing.T) { + cfg := config.Config{} + cfg.ExitCleanup = processtreecreatorconfig.ExitCleanupConfig{ + MaxPendingExits: 1000, CleanupInterval: time.Hour, CleanupDelay: time.Minute, + } + creator := NewProcessTreeCreator(&mockContainerProcessTree{}, cfg).(*processTreeCreatorImpl) + creator.readStartTime = func(pid uint32) (uint64, time.Time) { return 0, time.Time{} } // read fails + + wall := time.Date(2026, 8, 3, 9, 0, 0, 0, time.UTC) + creator.FeedEvent(conversion.ProcessEvent{ + Type: conversion.ProcfsEvent, PID: 100, PPID: 1, Comm: "nginx", + StartTimeNs: 1_000_000_000, StartTimeWall: wall, + }) + require.Equal(t, uint64(1_000_000_000), creator.GetProcessBootTimeNs(100)) + + // No exit for this pid, so the node is still nginx's — not a recycled pid. + creator.FeedEvent(conversion.ProcessEvent{Type: conversion.ForkEvent, PID: 100, PPID: 1, Comm: "nginx"}) + + assert.Equal(t, uint64(1_000_000_000), creator.GetProcessBootTimeNs(100), + "a failed read must not destroy a value the scan already recorded") + node, err := creator.GetProcessNode(100) + require.NoError(t, err) + assert.Equal(t, wall, node.StartTime, "nor the display value") +} diff --git a/pkg/processtree/feeder/procfs_feeder.go b/pkg/processtree/feeder/procfs_feeder.go index ab329127cc..180967e483 100644 --- a/pkg/processtree/feeder/procfs_feeder.go +++ b/pkg/processtree/feeder/procfs_feeder.go @@ -15,6 +15,15 @@ import ( "github.com/prometheus/procfs" ) +// ticksPerSecond is USER_HZ, the unit of /proc//stat field 22. Hardcoded to +// match prometheus/procfs's own userHZ constant and the wire contract's +// documentation that start times are exact multiples of 10 ms. +const ticksPerSecond = 100 + +// nsPerTick converts /proc//stat field 22 (clock ticks since boot) to +// boot-relative nanoseconds. Exact: 1e9 / 100 = 10,000,000. +const nsPerTick = uint64(1_000_000_000 / ticksPerSecond) + // ProcfsFeeder implements ProcessEventFeeder by reading process information from /proc filesystem. type ProcfsFeeder struct { subscribers []chan<- conversion.ProcessEvent @@ -26,6 +35,10 @@ type ProcfsFeeder struct { procfsPath string procfs procfs.FS processTreeManager processtree.ProcessTreeManager + // bootTime is /proc/stat's btime, read once at Start. Used only to derive the + // display-only wall-clock start time; it has whole-second resolution, which is + // why boot-relative nanoseconds remain the sole process-identity source. + bootTime time.Time } // procInfo is a helper struct to pass results from worker goroutines. @@ -61,6 +74,13 @@ func (pf *ProcfsFeeder) Start(ctx context.Context) error { } pf.procfs = fs + if stat, err := fs.Stat(); err == nil { + pf.bootTime = time.Unix(int64(stat.BootTime), 0) + } else { + // Wall-clock start times will stay zero; boot-relative identity is unaffected. + fmt.Fprintf(os.Stderr, "procfs feeder: failed to read btime, StartTimeWall disabled: %v\n", err) + } + // Create a cancellable context for graceful shutdown pf.ctx, pf.cancel = context.WithCancel(ctx) @@ -233,6 +253,14 @@ func (pf *ProcfsFeeder) readProcessInfo(pid uint32) (conversion.ProcessEvent, er event.PPID = uint32(stat.PPID) event.Comm = stat.Comm + // Field 22: process creation time in clock ticks since boot. Convert to + // boot-relative nanoseconds (the identity unit on the wire) and, separately, + // to a display-only wall-clock time via btime. + event.StartTimeNs = stat.Starttime * nsPerTick + if !pf.bootTime.IsZero() && event.StartTimeNs != 0 { + event.StartTimeWall = pf.bootTime.Add(time.Duration(event.StartTimeNs)) + } + if status, err := proc.NewStatus(); err == nil { uid := uint32(status.UIDs[1]) gid := uint32(status.GIDs[1]) diff --git a/pkg/processtree/feeder/procfs_feeder_test.go b/pkg/processtree/feeder/procfs_feeder_test.go index fca872b2a5..69b7cb2c02 100644 --- a/pkg/processtree/feeder/procfs_feeder_test.go +++ b/pkg/processtree/feeder/procfs_feeder_test.go @@ -8,6 +8,7 @@ import ( "github.com/kubescape/node-agent/pkg/processtree" "github.com/kubescape/node-agent/pkg/processtree/conversion" + "github.com/prometheus/procfs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -108,6 +109,45 @@ func TestProcfsFeeder_ReadProcessInfo(t *testing.T) { assert.Error(t, err) } +func TestProcfsFeeder_ReadProcessInfo_StartTime(t *testing.T) { + mockManager := processtree.NewProcessTreeManagerMock() + feeder := NewProcfsFeeder(100*time.Millisecond, 10*time.Millisecond, mockManager) + require.NoError(t, feeder.Start(context.Background())) + defer feeder.Stop() + + event, err := feeder.readProcessInfo(uint32(os.Getpid())) + require.NoError(t, err) + + // Boot-relative nanoseconds: non-zero and an exact multiple of 10ms, + // because /proc//stat field 22 is denominated in USER_HZ=100 ticks. + assert.NotZero(t, event.StartTimeNs) + assert.Zero(t, event.StartTimeNs%10_000_000, + "procfs-derived StartTimeNs must be an exact multiple of 10^7 ns") + + // Wall-clock derivation: btime + ticks/HZ. Sanity: within [boot, now]. + assert.False(t, event.StartTimeWall.IsZero()) + assert.True(t, event.StartTimeWall.Before(time.Now().Add(2*time.Second))) + // This process started after boot: wall - bootNs must land near btime. + // (Loose check: StartTimeWall minus the boot-relative duration is in the past.) + assert.True(t, event.StartTimeWall.Add(-time.Duration(event.StartTimeNs)).Before(time.Now())) + + // Pin the conversion against an independently read field 22 and the contract's + // literal 10^7, so a drifted ticksPerSecond fails deterministically instead of + // only on machines whose uptime happens to make the error visible. + p, err := procfs.NewProc(os.Getpid()) + require.NoError(t, err) + stat, err := p.Stat() + require.NoError(t, err) + assert.Equal(t, stat.Starttime*10_000_000, event.StartTimeNs, + "conversion must be exactly field 22 ticks * 10^7 ns (USER_HZ=100)") + + // Sharper check on the arithmetic itself: this is the test binary's own pid, + // so its real creation time is moments ago. A wrong btime or a wrong tick + // scaling would land this decades or hours away rather than minutes. + assert.WithinDuration(t, time.Now(), event.StartTimeWall, 5*time.Minute, + "btime + ticks/HZ must reconstruct the test process's actual start time") +} + func TestProcfsFeeder_GetProcessComm(t *testing.T) { mockManager := processtree.NewProcessTreeManagerMock() feeder := NewProcfsFeeder(100*time.Millisecond, 10*time.Millisecond, mockManager) diff --git a/pkg/processtree/process_tree_manager.go b/pkg/processtree/process_tree_manager.go index 989771754e..df8f87ac4b 100644 --- a/pkg/processtree/process_tree_manager.go +++ b/pkg/processtree/process_tree_manager.go @@ -109,6 +109,12 @@ func (ptm *ProcessTreeManagerImpl) GetContainerProcessTree(containerID string, p return containerSubtree, nil } +func (ptm *ProcessTreeManagerImpl) GetProcessBootTimeNs(pid uint32) uint64 { + ptm.mutex.RLock() + defer ptm.mutex.RUnlock() + return ptm.creator.GetProcessBootTimeNs(pid) +} + func (ptm *ProcessTreeManagerImpl) GetPidList() []uint32 { ptm.mutex.RLock() defer ptm.mutex.RUnlock() diff --git a/pkg/processtree/process_tree_manager_interface.go b/pkg/processtree/process_tree_manager_interface.go index fd98715bc2..aa11122288 100644 --- a/pkg/processtree/process_tree_manager_interface.go +++ b/pkg/processtree/process_tree_manager_interface.go @@ -11,4 +11,15 @@ type ProcessTreeManager interface { GetContainerProcessTree(containerID string, pid uint32, useCache bool) (armotypes.Process, error) ReportEvent(eventType utils.EventType, event utils.K8sEvent) error GetPidList() []uint32 + // GetProcessBootTimeNs returns the process's creation time as nanoseconds + // since boot (CLOCK_BOOTTIME), sourced exclusively from /proc//stat + // field 22 — either the periodic scan or the on-demand read performed when a + // fork/exec creates the node. Returns 0 when unknown: the process died + // before its creation event was processed, or it has already exited. + // + // This is the sole process-identity time source. The wall-clock + // armotypes.Process.StartTime carried on tree nodes is display-only and + // inherits btime's whole-second skew, so it must never be compared for + // identity. + GetProcessBootTimeNs(pid uint32) uint64 } diff --git a/pkg/processtree/process_tree_manager_mock.go b/pkg/processtree/process_tree_manager_mock.go index 4726f171fd..8d04552684 100644 --- a/pkg/processtree/process_tree_manager_mock.go +++ b/pkg/processtree/process_tree_manager_mock.go @@ -7,7 +7,8 @@ import ( // ProcessTreeManagerMock implements the ProcessTreeManager interface for testing type ProcessTreeManagerMock struct { - pidList []uint32 + pidList []uint32 + bootTimeNs map[uint32]uint64 } var _ ProcessTreeManager = (*ProcessTreeManagerMock)(nil) @@ -48,3 +49,17 @@ func (m *ProcessTreeManagerMock) ReportEvent(eventType utils.EventType, event ut func (m *ProcessTreeManagerMock) GetPidList() []uint32 { return m.pidList } + +// GetProcessBootTimeNs returns the configured start time for a pid, or 0 for an +// unknown one — matching the real manager's "0 means unknown" contract. +func (m *ProcessTreeManagerMock) GetProcessBootTimeNs(pid uint32) uint64 { + return m.bootTimeNs[pid] +} + +// SetProcessBootTimeNs sets the boot-relative start time the mock reports for a pid. +func (m *ProcessTreeManagerMock) SetProcessBootTimeNs(pid uint32, ns uint64) { + if m.bootTimeNs == nil { + m.bootTimeNs = make(map[uint32]uint64) + } + m.bootTimeNs[pid] = ns +} diff --git a/pkg/processtree/process_tree_manager_test.go b/pkg/processtree/process_tree_manager_test.go new file mode 100644 index 0000000000..ca4d039197 --- /dev/null +++ b/pkg/processtree/process_tree_manager_test.go @@ -0,0 +1,88 @@ +package processtree + +import ( + "os" + "testing" + "time" + + containercollection "github.com/inspektor-gadget/inspektor-gadget/pkg/container-collection" + eventtypes "github.com/inspektor-gadget/inspektor-gadget/pkg/types" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/ebpf/events" + containerprocesstree "github.com/kubescape/node-agent/pkg/processtree/container" + processtreecreator "github.com/kubescape/node-agent/pkg/processtree/creator" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestManager_GetProcessBootTimeNs exercises the accessor that the network-stream +// attribution work calls to build a per-connection process reference: a procfs +// event reported through the manager must surface its boot-relative start time. +func TestManager_GetProcessBootTimeNs(t *testing.T) { + creator := processtreecreator.NewProcessTreeCreator(containerprocesstree.NewContainerProcessTree(), config.Config{}) + mgr := NewProcessTreeManager(creator, containerprocesstree.NewContainerProcessTree(), config.Config{}) + + require.NoError(t, mgr.ReportEvent(utils.ProcfsEventType, &events.ProcfsEvent{ + PID: 77, PPID: 1, Comm: "worker", StartTimeNs: 2_340_000_000, + })) + + assert.Equal(t, uint64(2_340_000_000), mgr.GetProcessBootTimeNs(77)) + assert.Zero(t, mgr.GetProcessBootTimeNs(1234), "a pid the tree has never seen reads as 0") +} + +// TestManager_GetContainerProcessTree_CarriesStartTime is the end-to-end guard +// for the trap this change exists to avoid: the start time is populated on the +// creator's node, but every consumer reads it through a copy function that +// strips any field it does not explicitly list. This walks the real production +// path — procfs event -> ProcessTreeManager.ReportEvent -> creator -> +// containerprocesstree.buildBranchToShim -> GetContainerProcessTree — against a +// creator-populated tree rather than a hand-built fixture, so it fails if the +// branch builder ever stops carrying StartTime again. +// +// The container is registered through the real ContainerCallback, which derives +// the shim pid by reading the container pid's parent from /proc. That needs a +// pid that actually exists, so the test uses its own. +func TestManager_GetContainerProcessTree_CarriesStartTime(t *testing.T) { + containerTree := containerprocesstree.NewContainerProcessTree() + creator := processtreecreator.NewProcessTreeCreator(containerTree, config.Config{}) + mgr := NewProcessTreeManager(creator, containerTree, config.Config{}) + + self := uint32(os.Getpid()) + shim := uint32(os.Getppid()) + require.NotEqual(t, self, shim, "test needs a distinct parent pid") + + const containerID = "e2e-start-time-container" + containerTree.ContainerCallback(containercollection.PubSubEvent{ + Type: containercollection.EventTypeAddContainer, + Container: &containercollection.Container{ + Runtime: containercollection.RuntimeMetadata{ + BasicRuntimeMetadata: eventtypes.BasicRuntimeMetadata{ + ContainerID: containerID, + ContainerPID: self, + }, + }, + }, + }) + + shimWall := time.Date(2026, 7, 29, 9, 0, 0, 0, time.UTC) + selfWall := time.Date(2026, 7, 29, 9, 30, 0, 0, time.UTC) + + require.NoError(t, mgr.ReportEvent(utils.ProcfsEventType, &events.ProcfsEvent{ + PID: shim, PPID: 1, Comm: "containerd-shim", + StartTimeNs: 1_110_000_000, StartTimeWall: shimWall, + })) + require.NoError(t, mgr.ReportEvent(utils.ProcfsEventType, &events.ProcfsEvent{ + PID: self, PPID: shim, Comm: "nginx", + StartTimeNs: 2_220_000_000, StartTimeWall: selfWall, + })) + + branch, err := mgr.GetContainerProcessTree(containerID, self, false) + require.NoError(t, err) + assert.Equal(t, self, branch.PID) + assert.Equal(t, selfWall, branch.StartTime, + "the branch handed to alerts must carry the creator-populated StartTime") + + // The boot-relative identity for the same process, from the side map. + assert.Equal(t, uint64(2_220_000_000), mgr.GetProcessBootTimeNs(self)) +} diff --git a/pkg/utils/processtree_merge.go b/pkg/utils/processtree_merge.go index 1418a2517e..49002bbdab 100644 --- a/pkg/utils/processtree_merge.go +++ b/pkg/utils/processtree_merge.go @@ -53,6 +53,7 @@ func CopyProcess(src *armotypes.Process) *armotypes.Process { Cwd: src.Cwd, Gid: copyUint32Ptr(src.Gid), Uid: copyUint32Ptr(src.Uid), + StartTime: src.StartTime, ChildrenMap: make(map[armotypes.CommPID]*armotypes.Process), } } @@ -87,6 +88,9 @@ func EnrichProcess(target *armotypes.Process, source *armotypes.Process) { if target.Gid == nil && source.Gid != nil { target.Gid = copyUint32Ptr(source.Gid) } + if target.StartTime.IsZero() && !source.StartTime.IsZero() { + target.StartTime = source.StartTime + } } // copyUint32Ptr creates a copy of a uint32 pointer diff --git a/pkg/utils/processtree_merge_test.go b/pkg/utils/processtree_merge_test.go new file mode 100644 index 0000000000..afb7cc5ea5 --- /dev/null +++ b/pkg/utils/processtree_merge_test.go @@ -0,0 +1,28 @@ +package utils + +import ( + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/stretchr/testify/assert" +) + +func TestCopyProcess_CarriesStartTime(t *testing.T) { + wall := time.Date(2026, 7, 29, 10, 0, 0, 0, time.UTC) + src := &armotypes.Process{PID: 5, Comm: "a", StartTime: wall} + assert.Equal(t, wall, CopyProcess(src).StartTime, + "CopyProcess strips any field it does not explicitly list — StartTime must be listed") +} + +func TestEnrichProcess_FillsEmptyStartTime(t *testing.T) { + wall := time.Date(2026, 7, 29, 10, 0, 0, 0, time.UTC) + target := &armotypes.Process{PID: 5} + EnrichProcess(target, &armotypes.Process{PID: 5, StartTime: wall}) + assert.Equal(t, wall, target.StartTime) + + // fill-if-empty: an existing value is never overwritten + other := wall.Add(time.Hour) + EnrichProcess(target, &armotypes.Process{PID: 5, StartTime: other}) + assert.Equal(t, wall, target.StartTime) +}