From e2247f05529e2d37ed526334cb170b127c852ca1 Mon Sep 17 00:00:00 2001 From: mayankpande88 Date: Fri, 18 Sep 2026 18:07:07 +0530 Subject: [PATCH] fix(profiler): reach the target through its own mount namespace Files we stage for the target - and files the target writes for us - were resolved through the container runtime's overlay directory. That is only the same thing when nothing is mounted over the container's root. A pod with readOnlyRootFilesystem: true and an emptyDir at /tmp is the common counter-example, and there everything we put in the target's /tmp lands somewhere the process cannot see. Java flamegraphs failed exactly that way on such pods, with async-profiler reporting "Could not start attach mechanism: No such file or directory": both jattach's .attach_pid handshake file and libasyncProfiler.so, which the JVM dlopens, were written through the overlay. Resolve those paths via /proc//root instead, which is the target's whole mount namespace, volumes included - identical to the old path when nothing is mounted there, correct when something is. Same change for jcmd (heap dumps and JFR recordings are written by the JVM) and for the Node heapsnapshot (written by the Node process into its working directory). Also hand the staged profiler to the user the target runs as: we stage as root, and the JVM has to read the library and write its output there. --- internal/agent/profiler/jvm/async_profiler.go | 59 ++++++++++++------ .../agent/profiler/jvm/async_profiler_fake.go | 13 ++++ .../agent/profiler/jvm/async_profiler_test.go | 60 +++++++++++++++++-- internal/agent/profiler/jvm/jcmd.go | 40 ++++++------- internal/agent/profiler/jvm/jcmd_test.go | 6 +- internal/agent/profiler/node_dummy.go | 8 ++- internal/agent/profiler/node_dummy_test.go | 8 ++- internal/agent/util/container.go | 52 +++++++++++++++- internal/agent/util/target_test.go | 39 ++++++++++++ 9 files changed, 236 insertions(+), 49 deletions(-) create mode 100644 internal/agent/util/target_test.go diff --git a/internal/agent/profiler/jvm/async_profiler.go b/internal/agent/profiler/jvm/async_profiler.go index afd4daa..9114946 100644 --- a/internal/agent/profiler/jvm/async_profiler.go +++ b/internal/agent/profiler/jvm/async_profiler.go @@ -63,6 +63,7 @@ type AsyncProfilerManager interface { linkTmpDirToTargetTmpDir(string) error copyProfilerToTmpDir() error selectProfilerLibrary(string) error + chownProfilerToTarget(string) error invoke(*job.ProfilingJob, string) (error, time.Duration) cleanUp(*job.ProfilingJob, string) } @@ -83,14 +84,25 @@ func NewAsyncProfiler(commander executil.Commander, publisher publish.Publisher) } func (j *AsyncProfiler) SetUp(job *job.ProfilingJob) error { - targetFs, err := util.ContainerFileSystem(job.ContainerRuntime, job.ContainerID, job.ContainerRuntimePath) - if err != nil { - return err + // PIDs first: everything below is staged through the target's own mount + // namespace, which we can only reach via one of its PIDs. + if stringUtils.IsNotBlank(job.PID) { + j.targetPIDs = []string{job.PID} + } else { + pids, err := util.GetCandidatePIDs(job) + if err != nil { + return err + } + log.DebugLogLn(fmt.Sprintf("The PIDs to be profiled: %s", pids)) + j.targetPIDs = pids } + + // Every PID of a container shares its mount namespace, so any of them + // resolves the same filesystem. + targetFs := util.TargetRootFS(j.targetPIDs[0]) log.DebugLogLn(fmt.Sprintf("The target filesystem is: %s", targetFs)) - err = j.removeTmpDir() - if err != nil { + if err := j.removeTmpDir(); err != nil { return err } @@ -98,27 +110,21 @@ func (j *AsyncProfiler) SetUp(job *job.ProfilingJob) error { // remove previous files from a previous profiling file.RemoveAll(targetTmpDir, config.ProfilingPrefix+string(job.OutputType)) - err = j.linkTmpDirToTargetTmpDir(targetTmpDir) - if err != nil { + if err := j.linkTmpDirToTargetTmpDir(targetTmpDir); err != nil { return err } - if stringUtils.IsNotBlank(job.PID) { - j.targetPIDs = []string{job.PID} - } else { - pids, err := util.GetCandidatePIDs(job) - if err != nil { - return err - } - log.DebugLogLn(fmt.Sprintf("The PIDs to be profiled: %s", pids)) - j.targetPIDs = pids + if err := j.copyProfilerToTmpDir(); err != nil { + return err } - if err := j.copyProfilerToTmpDir(); err != nil { + if err := j.selectProfilerLibrary(targetFs); err != nil { return err } - return j.selectProfilerLibrary(targetFs) + // The JVM dlopens the library and writes the profile itself, as whatever + // user it runs as — root-owned staging is unreadable/unwritable for it. + return j.chownProfilerToTarget(j.targetPIDs[0]) } // targetUsesMusl reports whether the target container's root filesystem is @@ -151,6 +157,23 @@ func (j *asyncProfilerManager) copyProfilerToTmpDir() error { return cmd.Run() } +// chownProfilerToTarget hands the staged directory to the user the target runs +// as. We stage as root; the JVM then has to read libasyncProfiler.so and write +// its own output file into that directory, and most hardened images do not run +// as root. +func (j *asyncProfilerManager) chownProfilerToTarget(pid string) error { + uid, gid, err := util.TargetCredentials(pid) + if err != nil { + return err + } + if uid == "0" && gid == "0" { + return nil + } + log.DebugLogLn(fmt.Sprintf("Handing the staged profiler to %s:%s", uid, gid)) + cmd := j.commander.Command("chown", "-R", uid+":"+gid, asyncProfilerDir) + return cmd.Run() +} + // selectProfilerLibrary points libasyncProfiler.so at the build matching the // target's libc. The library is dlopen'd by the target JVM rather than by us, // so a mismatch fails the attach with "libc.musl-x86_64.so.1: cannot open diff --git a/internal/agent/profiler/jvm/async_profiler_fake.go b/internal/agent/profiler/jvm/async_profiler_fake.go index 73ef552..08aa198 100644 --- a/internal/agent/profiler/jvm/async_profiler_fake.go +++ b/internal/agent/profiler/jvm/async_profiler_fake.go @@ -128,6 +128,19 @@ func (f *fakeAsyncProfilerManager) selectProfilerLibrary(s string) error { return err } +func (f *fakeAsyncProfilerManager) chownProfilerToTarget(s string) error { + var err error + f.fakeMethods["chownProfilerToTarget"].invokes++ + if f.fakeMethods["chownProfilerToTarget"].fakeReturnValues != nil && len(f.fakeMethods["chownProfilerToTarget"].fakeReturnValues) > 0 { + f.fakeMethods["chownProfilerToTarget"].indexExecution++ + arg0 := f.fakeMethods["chownProfilerToTarget"].fakeReturnValues[f.fakeMethods["chownProfilerToTarget"].indexExecution-1].([]interface{})[0] + if arg0 != nil { + err = arg0.(error) + } + } + return err +} + func (f *fakeAsyncProfilerManager) cleanUp(profilingJob *job.ProfilingJob, s string) { f.fakeMethods["cleanUp"].invokes++ if f.fakeMethods["cleanUp"].fakeReturnValues != nil && len(f.fakeMethods["cleanUp"].fakeReturnValues) > 0 { diff --git a/internal/agent/profiler/jvm/async_profiler_test.go b/internal/agent/profiler/jvm/async_profiler_test.go index 33cc3cc..215fac8 100644 --- a/internal/agent/profiler/jvm/async_profiler_test.go +++ b/internal/agent/profiler/jvm/async_profiler_test.go @@ -43,6 +43,7 @@ func TestAsyncProfiler_SetUp(t *testing.T) { asyncProfilerManager.On("linkTmpDirToTargetTmpDir").Return(nil) asyncProfilerManager.On("copyProfilerToTmpDir").Return(nil) asyncProfilerManager.On("selectProfilerLibrary").Return(nil) + asyncProfilerManager.On("chownProfilerToTarget").Return(nil) return fields{ AsyncProfiler: &AsyncProfiler{ @@ -75,6 +76,7 @@ func TestAsyncProfiler_SetUp(t *testing.T) { asyncProfilerManager.On("linkTmpDirToTargetTmpDir").Return(nil) asyncProfilerManager.On("copyProfilerToTmpDir").Return(nil) asyncProfilerManager.On("selectProfilerLibrary").Return(nil) + asyncProfilerManager.On("chownProfilerToTarget").Return(nil) return fields{ AsyncProfiler: &AsyncProfiler{ @@ -101,13 +103,14 @@ func TestAsyncProfiler_SetUp(t *testing.T) { }, }, { - name: "should fail when getting target filesystem fail", + name: "should fail when the container runtime is unknown", given: func() (fields, args) { asyncProfilerManager := newFakeAsyncProfilerManager() asyncProfilerManager.On("removeTmpDir").Return(nil) asyncProfilerManager.On("linkTmpDirToTargetTmpDir").Return(nil) asyncProfilerManager.On("copyProfilerToTmpDir").Return(nil) asyncProfilerManager.On("selectProfilerLibrary").Return(nil) + asyncProfilerManager.On("chownProfilerToTarget").Return(nil) return fields{ AsyncProfiler: &AsyncProfiler{ @@ -118,7 +121,6 @@ func TestAsyncProfiler_SetUp(t *testing.T) { Duration: 0, ContainerRuntime: "other", ContainerID: "ContainerID", - PID: "PID_ContainerID", }, } }, @@ -197,6 +199,8 @@ func TestAsyncProfiler_SetUp(t *testing.T) { asyncProfilerManager := newFakeAsyncProfilerManager() asyncProfilerManager.On("removeTmpDir").Return(nil) asyncProfilerManager.On("linkTmpDirToTargetTmpDir").Return(nil) + // PIDs are resolved first now — the target's mount namespace is + // reached through one of them, so nothing can be staged before. return fields{ AsyncProfiler: &AsyncProfiler{ @@ -215,8 +219,8 @@ func TestAsyncProfiler_SetUp(t *testing.T) { }, then: func(t *testing.T, err error, fields fields) { assert.NotNil(t, err) - assert.Equal(t, 1, fields.AsyncProfiler.AsyncProfilerManager.(FakeAsyncProfilerManager).On("removeTmpDir").InvokedTimes()) - assert.Equal(t, 1, fields.AsyncProfiler.AsyncProfilerManager.(FakeAsyncProfilerManager).On("linkTmpDirToTargetTmpDir").InvokedTimes()) + assert.Equal(t, 0, fields.AsyncProfiler.AsyncProfilerManager.(FakeAsyncProfilerManager).On("removeTmpDir").InvokedTimes()) + assert.Equal(t, 0, fields.AsyncProfiler.AsyncProfilerManager.(FakeAsyncProfilerManager).On("linkTmpDirToTargetTmpDir").InvokedTimes()) assert.Equal(t, 0, fields.AsyncProfiler.AsyncProfilerManager.(FakeAsyncProfilerManager).On("copyProfilerToTmpDir").InvokedTimes()) }, }, @@ -495,6 +499,54 @@ func Test_asyncProfilerManager_selectProfilerLibrary(t *testing.T) { }) } +// Test_asyncProfilerManager_chownProfilerToTarget — we stage as root, but the +// JVM reads the library and writes its own output into that directory as +// whatever user it runs as, which in hardened images is not root. +func Test_asyncProfilerManager_chownProfilerToTarget(t *testing.T) { + t.Run("a root target needs no chown", func(t *testing.T) { + requireProcFS(t) + if os.Getuid() != 0 { + t.Skip("this test reads our own /proc entry, so it only says root when we are") + } + commander := executil.NewFakeCommander() + commander.On("Command").Return(exec.Command("false")) + a := NewAsyncProfiler(commander, publish.NewFakePublisher()) + + assert.Nil(t, a.chownProfilerToTarget("self")) + assert.Equal(t, 0, commander.On("Command").InvokedTimes()) + }) + + t.Run("a non-root target is handed the staged directory", func(t *testing.T) { + requireProcFS(t) + if os.Getuid() == 0 { + t.Skip("running as root, so our own /proc entry cannot stand in for a non-root target") + } + commander := executil.NewFakeCommander() + commander.On("Command").Return(exec.Command("true")) + a := NewAsyncProfiler(commander, publish.NewFakePublisher()) + + assert.Nil(t, a.chownProfilerToTarget("self")) + assert.Equal(t, 1, commander.On("Command").InvokedTimes()) + }) + + t.Run("an unreadable pid is an error, not a silent skip", func(t *testing.T) { + commander := executil.NewFakeCommander() + commander.On("Command").Return(exec.Command("true")) + a := NewAsyncProfiler(commander, publish.NewFakePublisher()) + + assert.NotNil(t, a.chownProfilerToTarget("not-a-pid")) + }) +} + +// requireProcFS skips on platforms without /proc — the credentials come from +// /proc//status, which only exists on the Linux boxes this runs on. +func requireProcFS(t *testing.T) { + t.Helper() + if _, err := os.Stat("/proc/self/status"); err != nil { + t.Skip("no procfs on this platform") + } +} + func Test_asyncProfilerManager_invoke(t *testing.T) { type fields struct { AsyncProfiler *AsyncProfiler diff --git a/internal/agent/profiler/jvm/jcmd.go b/internal/agent/profiler/jvm/jcmd.go index c6f354a..2b374df 100644 --- a/internal/agent/profiler/jvm/jcmd.go +++ b/internal/agent/profiler/jvm/jcmd.go @@ -96,26 +96,8 @@ func NewJcmdProfiler(commander executil.Commander, publisher publish.Publisher) } func (j *JcmdProfiler) SetUp(job *job.ProfilingJob) error { - targetFs, err := util.ContainerFileSystem(job.ContainerRuntime, job.ContainerID, job.ContainerRuntimePath) - if err != nil { - return err - } - log.DebugLogLn(fmt.Sprintf("The target filesystem is: %s", targetFs)) - - err = j.removeTmpDir() - if err != nil { - return err - } - - targetTmpDir := filepath.Join(targetFs, "tmp") - // remove previous files from a previous profiling - file.RemoveAll(targetTmpDir, config.ProfilingPrefix+string(job.OutputType)) - - err = j.linkTmpDirToTargetTmpDir(targetTmpDir) - if err != nil { - return err - } - + // PIDs first: the tmp dir below is the target's own, reachable only + // through one of its PIDs. if stringUtils.IsNotBlank(job.PID) { j.targetPIDs = []string{job.PID} recordingPIDs = make(chan string, 1) @@ -129,6 +111,24 @@ func (j *JcmdProfiler) SetUp(job *job.ProfilingJob) error { recordingPIDs = make(chan string, len(pids)) } + // The JVM writes heap dumps and JFR recordings itself, to a path in its own + // mount namespace — so we have to agree with it on what /tmp is. Every PID + // of a container shares that namespace. + targetFs := util.TargetRootFS(j.targetPIDs[0]) + log.DebugLogLn(fmt.Sprintf("The target filesystem is: %s", targetFs)) + + if err := j.removeTmpDir(); err != nil { + return err + } + + targetTmpDir := filepath.Join(targetFs, "tmp") + // remove previous files from a previous profiling + file.RemoveAll(targetTmpDir, config.ProfilingPrefix+string(job.OutputType)) + + if err := j.linkTmpDirToTargetTmpDir(targetTmpDir); err != nil { + return err + } + return j.copyJfrSettingsToTmpDir() } diff --git a/internal/agent/profiler/jvm/jcmd_test.go b/internal/agent/profiler/jvm/jcmd_test.go index a32b2fb..8ca06b7 100644 --- a/internal/agent/profiler/jvm/jcmd_test.go +++ b/internal/agent/profiler/jvm/jcmd_test.go @@ -210,8 +210,10 @@ func TestJcmdProfiler_SetUp(t *testing.T) { }, then: func(t *testing.T, err error, fields fields) { assert.NotNil(t, err) - assert.Equal(t, 1, fields.JcmdProfiler.JcmdManager.(FakeJcmdManager).On("removeTmpDir").InvokedTimes()) - assert.Equal(t, 1, fields.JcmdProfiler.JcmdManager.(FakeJcmdManager).On("linkTmpDirToTargetTmpDir").InvokedTimes()) + // PIDs are resolved first now — the target's /tmp is reached + // through one of them, so nothing is staged before that. + assert.Equal(t, 0, fields.JcmdProfiler.JcmdManager.(FakeJcmdManager).On("removeTmpDir").InvokedTimes()) + assert.Equal(t, 0, fields.JcmdProfiler.JcmdManager.(FakeJcmdManager).On("linkTmpDirToTargetTmpDir").InvokedTimes()) assert.Equal(t, 0, fields.JcmdProfiler.JcmdManager.(FakeJcmdManager).On("copyJfrSettingsToTmpDir").InvokedTimes()) }, }, diff --git a/internal/agent/profiler/node_dummy.go b/internal/agent/profiler/node_dummy.go index b3f73c0..13efedc 100644 --- a/internal/agent/profiler/node_dummy.go +++ b/internal/agent/profiler/node_dummy.go @@ -45,10 +45,16 @@ func NewNodeDummyProfiler(publisher publish.Publisher) *NodeDummyProfiler { } func (n *NodeDummyProfiler) SetUp(job *job.ProfilingJob) error { - targetFs, err := util.ContainerFileSystem(job.ContainerRuntime, job.ContainerID, job.ContainerRuntimePath) + rootPID, err := util.GetRootPID(job) if err != nil { return err } + + // The heapsnapshot is written by the Node process into its working + // directory, so we have to read it back from the target's own mount + // namespace — the runtime's overlay path misses it whenever that + // directory is a volume. + targetFs := util.TargetRootFS(rootPID) log.DebugLogLn(fmt.Sprintf("The target filesystem is: %s", targetFs)) cwd, err := util.GetCWD(job) diff --git a/internal/agent/profiler/node_dummy_test.go b/internal/agent/profiler/node_dummy_test.go index 2239e8a..fc0ca42 100644 --- a/internal/agent/profiler/node_dummy_test.go +++ b/internal/agent/profiler/node_dummy_test.go @@ -53,11 +53,13 @@ func TestNodeDummyProfiler_SetUp(t *testing.T) { }, then: func(t *testing.T, err error, fields fields) { assert.Nil(t, err) - assert.Equal(t, "/root/fs/ContainerID/cwd", fields.NodeDummyProfiler.cwd) + // The heapsnapshot is read back through the target's own mount + // namespace, not the runtime's overlay directory. + assert.Equal(t, "/proc/PID_ContainerID/root/cwd", fields.NodeDummyProfiler.cwd) }, }, { - name: "should fail when get root file system fail", + name: "should fail when the container PID is not found", given: func() (fields, args) { return fields{ NodeDummyProfiler: &NodeDummyProfiler{ @@ -66,7 +68,7 @@ func TestNodeDummyProfiler_SetUp(t *testing.T) { }, args{ job: &job.ProfilingJob{ Duration: 0, - ContainerRuntime: api.FakeContainerWithRootFileSystemLocationResultError, + ContainerRuntime: api.FakeContainerWithPIDResultError, ContainerID: "ContainerID", }, } diff --git a/internal/agent/util/container.go b/internal/agent/util/container.go index 3b36ecc..926a727 100644 --- a/internal/agent/util/container.go +++ b/internal/agent/util/container.go @@ -3,6 +3,8 @@ package util import ( "bytes" "fmt" + "os" + "path/filepath" "regexp" "strings" "time" @@ -60,7 +62,55 @@ var runtime = func(r api.ContainerRuntime) (Container, error) { var commander = exec.NewCommander() -// ContainerFileSystem returns the root path of the container filesystem +// TargetRootFS returns the path through which we can reach the target's +// filesystem *as the target itself sees it*: /proc//root resolves the +// process's whole mount namespace, volumes included. +// +// Prefer this over ContainerFileSystem for anything the target has to read or +// write. The two agree only when nothing is mounted over the container's root; +// a pod with readOnlyRootFilesystem: true and an emptyDir at /tmp is the common +// counter-example, and there a file written through the runtime's overlay +// directory never becomes visible inside the container. +func TargetRootFS(pid string) string { + return filepath.Join("/proc", pid, "root") +} + +// TargetCredentials returns the uid and gid the target process runs as, from +// /proc//status. Files we stage for it to read — and the directory it +// writes its own output into — have to belong to it: the debugger pod is root, +// the target frequently is not. +func TargetCredentials(pid string) (uid string, gid string, err error) { + status, err := os.ReadFile(filepath.Join("/proc", pid, "status")) //nolint:gosec // pid comes from the container runtime + if err != nil { + return "", "", err + } + for _, line := range strings.Split(string(status), "\n") { + // "Uid:\t\t\t\t" — the real id is the one + // the process runs as, which is what file ownership has to match. + fields := strings.Fields(line) + if len(fields) < 2 { + continue + } + switch fields[0] { + case "Uid:": + uid = fields[1] + case "Gid:": + gid = fields[1] + } + } + if uid == "" || gid == "" { + return "", "", errors.Errorf("could not read uid/gid of PID %s from /proc", pid) + } + return uid, gid, nil +} + +// ContainerFileSystem returns the container runtime's own root path for the +// container — the overlay directory. +// +// NOTE: this is NOT what the container sees. Any path mounted over +// (an emptyDir at /tmp, a PVC, a projected secret) resolves differently inside +// the container's mount namespace, so reads and writes through here are +// invisible to the process. Use TargetRootFS for anything the target touches. func ContainerFileSystem(r api.ContainerRuntime, containerID string, containerRuntimePath string) (string, error) { if r == "" || containerID == "" { return "", errors.New(ContainerRuntimeAndContainerIdMandatoryText) diff --git a/internal/agent/util/target_test.go b/internal/agent/util/target_test.go new file mode 100644 index 0000000..ed04380 --- /dev/null +++ b/internal/agent/util/target_test.go @@ -0,0 +1,39 @@ +package util + +import ( + "os" + "path/filepath" + "strconv" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTargetRootFS locks the path we reach the target through. The runtime's +// overlay directory is NOT equivalent: anything mounted over the container's +// root (an emptyDir at /tmp is the common one) resolves differently inside the +// container, and a file staged through the overlay is invisible to it. +func TestTargetRootFS(t *testing.T) { + assert.Equal(t, "/proc/1234/root", TargetRootFS("1234")) + assert.Equal(t, "/proc/1234/root/tmp", filepath.Join(TargetRootFS("1234"), "tmp")) +} + +// TestTargetCredentials — we stage as root, but the target reads the library +// and writes its own output, so ownership has to follow the target. +func TestTargetCredentials(t *testing.T) { + t.Run("reads our own process", func(t *testing.T) { + if _, err := os.Stat("/proc/self/status"); err != nil { + t.Skip("no procfs on this platform") + } + uid, gid, err := TargetCredentials("self") + require.NoError(t, err) + assert.Equal(t, strconv.Itoa(os.Getuid()), uid) + assert.Equal(t, strconv.Itoa(os.Getgid()), gid) + }) + + t.Run("unknown pid", func(t *testing.T) { + _, _, err := TargetCredentials("not-a-pid") + assert.Error(t, err) + }) +}