Skip to content

Commit ef95b89

Browse files
committed
[EBPF] Exclude Host Profiler from library uprobes
1 parent 2d00390 commit ef95b89

2 files changed

Lines changed: 120 additions & 46 deletions

File tree

pkg/ebpf/uprobes/attacher.go

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ var (
5555
// ErrNoMatchingRule is returned when no rule matches the shared library path.
5656
ErrNoMatchingRule = errors.New("no matching rule")
5757
// regex that defines internal DataDog processes
58-
internalProcessRegex = regexp.MustCompile("datadog-agent/.*/((process|security|trace|otel)-agent|system-probe|agent)")
58+
internalProcessRegex = regexp.MustCompile("datadog-agent/.*/((process|security|trace|otel)-agent|host-profiler|system-probe|agent)")
5959
)
6060

6161
// AttachTarget defines the target to which we should attach the probes, libraries or executables
@@ -703,6 +703,29 @@ func (ua *UprobeAttacher) buildRegisterCallbacks(matchingRules []*AttachRule, pr
703703
return registerCB, unregisterCB
704704
}
705705

706+
func resolveExecutable(procInfo *ProcInfo) (string, error) {
707+
binPath, err := procInfo.Exe()
708+
if err == nil || errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ESRCH) {
709+
return binPath, err
710+
}
711+
return "", utils.NewUnknownAttachmentError(err)
712+
}
713+
714+
func (ua *UprobeAttacher) rejectInternalProcess(procInfo *ProcInfo) error {
715+
if (ua.config.ExcludeTargets & ExcludeInternal) == 0 {
716+
return nil
717+
}
718+
719+
binPath, err := resolveExecutable(procInfo)
720+
if err != nil {
721+
return err
722+
}
723+
if internalProcessRegex.MatchString(binPath) {
724+
return ErrInternalDDogProcessRejected
725+
}
726+
return nil
727+
}
728+
706729
// AttachLibrary attaches the probes to the given library, opened by a given PID
707730
func (ua *UprobeAttacher) AttachLibrary(path string, pid uint32) (err error) {
708731
defer func() {
@@ -722,7 +745,12 @@ func (ua *UprobeAttacher) AttachLibrary(path string, pid uint32) (err error) {
722745
return ErrNoMatchingRule
723746
}
724747

725-
registerCB, unregisterCB := ua.buildRegisterCallbacks(matchingRules, NewProcInfo(ua.config.ProcRoot, pid))
748+
procInfo := NewProcInfo(ua.config.ProcRoot, pid)
749+
if err := ua.rejectInternalProcess(procInfo); err != nil {
750+
return err
751+
}
752+
753+
registerCB, unregisterCB := ua.buildRegisterCallbacks(matchingRules, procInfo)
726754

727755
return ua.fileRegistry.Register(path, pid, registerCB, unregisterCB, utils.IgnoreCB)
728756
}
@@ -751,15 +779,6 @@ func (ua *UprobeAttacher) getRulesForExecutable(path string, procInfo *ProcInfo)
751779
return matchedRules
752780
}
753781

754-
// getExecutablePath resolves the executable of the given PID looking in procfs.
755-
// Will return an error if the path cannot be resolved
756-
func (ua *UprobeAttacher) getExecutablePath(pid uint32) (string, error) {
757-
pidAsStr := strconv.FormatUint(uint64(pid), 10)
758-
exePath := filepath.Join(ua.config.ProcRoot, pidAsStr, "exe")
759-
760-
return os.Readlink(exePath)
761-
}
762-
763782
const optionAttachToLibs = true
764783

765784
// AttachPID attaches the corresponding probes to a given pid
@@ -781,28 +800,17 @@ func (ua *UprobeAttacher) AttachPIDWithOptions(pid uint32, attachToLibs bool) (e
781800
}
782801

783802
procInfo := NewProcInfo(ua.config.ProcRoot, pid)
803+
if err := ua.rejectInternalProcess(procInfo); err != nil {
804+
return err
805+
}
784806

785-
// Only compute the binary path if we are going to need it. It's better to do these two checks
786-
// (which are cheap, the handlesExecutables function is cached) than to do the syscall
787-
// every time
788807
var binPath string
789-
if ua.handlesExecutables() || (ua.config.ExcludeTargets&ExcludeInternal) != 0 {
790-
binPath, err = procInfo.Exe()
808+
if ua.handlesExecutables() {
809+
binPath, err = resolveExecutable(procInfo)
791810
if err != nil {
792-
// procfs can return ESRCH if the process exits while the kernel is
793-
// resolving /proc/<pid>/exe, even after path lookup has found it.
794-
if !errors.Is(err, os.ErrNotExist) && !errors.Is(err, syscall.ESRCH) {
795-
return utils.NewUnknownAttachmentError(err)
796-
}
797811
return err
798812
}
799-
}
800813

801-
if (ua.config.ExcludeTargets&ExcludeInternal) != 0 && internalProcessRegex.MatchString(binPath) {
802-
return ErrInternalDDogProcessRejected
803-
}
804-
805-
if ua.handlesExecutables() {
806814
matchingRules := ua.getRulesForExecutable(binPath, procInfo)
807815
if len(matchingRules) != 0 {
808816
registerCB, unregisterCB := ua.buildRegisterCallbacks(matchingRules, procInfo)

pkg/ebpf/uprobes/attacher_test.go

Lines changed: 85 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,91 @@ func TestInternalProcessesRegex(t *testing.T) {
5151
require.True(t, internalProcessRegex.MatchString("datadog-agent/bin/process-agent"))
5252
require.True(t, internalProcessRegex.MatchString("datadog-agent/bin/security-agent"))
5353
require.True(t, internalProcessRegex.MatchString("datadog-agent/bin/otel-agent"))
54+
require.True(t, internalProcessRegex.MatchString("/opt/datadog-agent/embedded/bin/host-profiler"))
55+
require.False(t, internalProcessRegex.MatchString("/opt/customer/bin/host-profiler"))
56+
}
57+
58+
func TestAttachLibraryHonorsExcludeInternal(t *testing.T) {
59+
const (
60+
pid = uint32(1)
61+
libPath = "/usr/lib/libssl.so.3"
62+
)
63+
64+
tests := []struct {
65+
name string
66+
exe string
67+
excludeTargets ExcludeMode
68+
createProcess bool
69+
expectedError error
70+
expectRegistration bool
71+
}{
72+
{
73+
name: "host profiler is excluded",
74+
exe: "/opt/datadog-agent/embedded/bin/host-profiler",
75+
excludeTargets: ExcludeInternal,
76+
createProcess: true,
77+
expectedError: ErrInternalDDogProcessRejected,
78+
},
79+
{
80+
name: "non-internal process is registered",
81+
exe: "/usr/bin/curl",
82+
excludeTargets: ExcludeInternal,
83+
createProcess: true,
84+
expectRegistration: true,
85+
},
86+
{
87+
name: "internal exclusion is opt-in",
88+
exe: "/opt/datadog-agent/embedded/bin/host-profiler",
89+
createProcess: true,
90+
expectRegistration: true,
91+
},
92+
{
93+
name: "process no longer exists",
94+
excludeTargets: ExcludeInternal,
95+
expectedError: os.ErrNotExist,
96+
},
97+
}
98+
99+
for _, tt := range tests {
100+
t.Run(tt.name, func(t *testing.T) {
101+
var entries []kernel.FakeProcFSEntry
102+
if tt.createProcess {
103+
entries = append(entries, kernel.FakeProcFSEntry{Pid: pid, Cmdline: tt.exe, Command: tt.exe, Exe: tt.exe})
104+
}
105+
procRoot := kernel.CreateFakeProcFS(t, entries)
106+
107+
config := AttacherConfig{
108+
ProcRoot: procRoot,
109+
ExcludeTargets: tt.excludeTargets,
110+
Rules: []*AttachRule{
111+
{
112+
Targets: AttachToSharedLibraries,
113+
LibraryNameRegex: regexp.MustCompile(`libssl\.so`),
114+
},
115+
},
116+
SharedLibsLibsets: []sharedlibraries.Libset{sharedlibraries.LibsetCrypto},
117+
}
118+
ua, err := NewUprobeAttacher(testModuleName, testAttacherName, config, &MockManager{}, nil, AttacherDependencies{ProcessMonitor: newMockProcessMonitor()})
119+
require.NoError(t, err)
120+
121+
registry := &MockFileRegistry{}
122+
if tt.expectRegistration {
123+
registry.On("Register", libPath, pid, mock.Anything, mock.Anything).Return(nil).Once()
124+
}
125+
ua.fileRegistry = registry
126+
127+
err = ua.AttachLibrary(libPath, pid)
128+
if tt.expectedError == nil {
129+
require.NoError(t, err)
130+
} else {
131+
require.ErrorIs(t, err, tt.expectedError)
132+
}
133+
registry.AssertExpectations(t)
134+
if !tt.expectRegistration {
135+
registry.AssertNotCalled(t, "Register", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
136+
}
137+
})
138+
}
54139
}
55140

56141
func TestAttachPidReturnsCorrectErrors(t *testing.T) {
@@ -413,25 +498,6 @@ func TestAttachToBinaryContainerdTmpReturnsErrEnvironment(t *testing.T) {
413498
require.ErrorIs(t, err, utils.ErrEnvironment)
414499
}
415500

416-
func TestGetExecutablePath(t *testing.T) {
417-
exe := "/bin/bash"
418-
procRoot := kernel.CreateFakeProcFS(t, []kernel.FakeProcFSEntry{{Pid: 1, Cmdline: "", Command: exe, Exe: exe}})
419-
config := AttacherConfig{
420-
ProcRoot: procRoot,
421-
}
422-
ua, err := NewUprobeAttacher(testModuleName, testAttacherName, config, &MockManager{}, nil, AttacherDependencies{ProcessMonitor: newMockProcessMonitor()})
423-
require.NoError(t, err)
424-
require.NotNil(t, ua)
425-
426-
path, err := ua.getExecutablePath(1)
427-
require.NoError(t, err, "failed to get executable path for existing PID")
428-
require.Equal(t, path, exe)
429-
430-
path, err = ua.getExecutablePath(404)
431-
require.Error(t, err, "should fail to get executable path for non-existing PID")
432-
require.Empty(t, path, "should return empty path for non-existing PID")
433-
}
434-
435501
const mapsFileSample = `
436502
08048000-08049000 r-xp 00000000 03:00 8312 /opt/test
437503
08049000-0804a000 rw-p 00001000 03:00 8312 /opt/test

0 commit comments

Comments
 (0)