[windows][cws][wkint-494] fix long, slow memory leak in CWS due to missed notifi…

pkg/config/setup/system_probe_cws.go

@@ -38,6 +38,9 @@ func initCWSSystemProbeConfig(cfg pkgconfigmodel.Config) {
  cfg.BindEnvAndSetDefault("runtime_security_config.use_secruntime_track", false)
  cfg.BindEnvAndSetDefault("runtime_security_config.compliance_module.enabled", false)
 
+ cfg.SetDefault("runtime_security_config.windows_filename_cache_max", 16384)
+ cfg.SetDefault("runtime_security_config.windows_registry_cache_max", 4096)
+
  // CWS - activity dump
  cfg.BindEnvAndSetDefault("runtime_security_config.activity_dump.enabled", true)
  cfg.BindEnvAndSetDefault("runtime_security_config.activity_dump.cleanup_period", "30s")

pkg/security/config/config.go

@@ -224,6 +224,12 @@ type RuntimeSecurityConfig struct {
 
  // Enforcement capabilities
  EnforcementEnabled bool
+
+ //WindowsFilenameCacheSize is the max number of filenames to cache
+ WindowsFilenameCacheSize int
+
+ //WindowsRegistryCacheSize is the max number of registry paths to cache
+ WindowsRegistryCacheSize int
 }
 
 // Config defines a security config
@@ -277,8 +283,10 @@ func NewRuntimeSecurityConfig() (*RuntimeSecurityConfig, error) {
  }
 
  rsConfig := &RuntimeSecurityConfig{
- RuntimeEnabled: coreconfig.SystemProbe.GetBool("runtime_security_config.enabled"),
- FIMEnabled: coreconfig.SystemProbe.GetBool("runtime_security_config.fim_enabled"),
+ RuntimeEnabled: coreconfig.SystemProbe.GetBool("runtime_security_config.enabled"),
+ FIMEnabled: coreconfig.SystemProbe.GetBool("runtime_security_config.fim_enabled"),
+ WindowsFilenameCacheSize: coreconfig.SystemProbe.GetInt("runtime_security_config.windows_filename_cache_max"),
+ WindowsRegistryCacheSize: coreconfig.SystemProbe.GetInt("runtime_security_config.windows_registry_cache_max"),
 
  SocketPath: coreconfig.SystemProbe.GetString("runtime_security_config.socket"),
  EventServerBurst: coreconfig.SystemProbe.GetInt("runtime_security_config.event_server.burst"),

pkg/security/probe/probe_kernel_file_windows.go

@@ -170,9 +170,8 @@ func (wp *WindowsProbe) parseCreateHandleArgs(e *etw.DDEventRecord) (*createHand
  return nil, errDiscardedPath
  }
 
- wp.filePathResolverLock.Lock()
- defer wp.filePathResolverLock.Unlock()
- wp.filePathResolver[ca.fileObject] = ca.fileName
+ // lru is thread safe, has its own locking
+ wp.filePathResolver.Add(ca.fileObject, fileCache{fileName: ca.fileName})
 
  return ca, nil
 }
@@ -267,10 +266,9 @@ func (wp *WindowsProbe) parseInformationArgs(e *etw.DDEventRecord) (*setInformat
  return nil, fmt.Errorf("unknown version number %v", e.EventHeader.EventDescriptor.Version)
  }
 
- wp.filePathResolverLock.Lock()
- defer wp.filePathResolverLock.Unlock()
- if s, ok := wp.filePathResolver[fileObjectPointer(sia.fileObject)]; ok {
- sia.fileName = s
+ // lru is thread safe, has its own locking
+ if s, ok := wp.filePathResolver.Get(fileObjectPointer(sia.fileObject)); ok {
+ sia.fileName = s.fileName
  }
 
  return sia, nil
@@ -402,10 +400,9 @@ func (wp *WindowsProbe) parseCleanupArgs(e *etw.DDEventRecord) (*cleanupArgs, er
  return nil, fmt.Errorf("unknown version number %v", e.EventHeader.EventDescriptor.Version)
  }
 
- wp.filePathResolverLock.Lock()
- defer wp.filePathResolverLock.Unlock()
- if s, ok := wp.filePathResolver[ca.fileObject]; ok {
- ca.fileName = s
+ // lru is thread safe, has its own locking
+ if s, ok := wp.filePathResolver.Get(fileObjectPointer(ca.fileObject)); ok {
+ ca.fileName = s.fileName
  }
 
  return ca, nil
@@ -495,10 +492,9 @@ func (wp *WindowsProbe) parseReadArgs(e *etw.DDEventRecord) (*readArgs, error) {
  } else {
  return nil, fmt.Errorf("unknown version number %v", e.EventHeader.EventDescriptor.Version)
  }
- wp.filePathResolverLock.Lock()
- defer wp.filePathResolverLock.Unlock()
- if s, ok := wp.filePathResolver[fileObjectPointer(ra.fileObject)]; ok {
- ra.fileName = s
+ // lru is thread safe, has its own locking
+ if s, ok := wp.filePathResolver.Get(fileObjectPointer(ra.fileObject)); ok {
+ ra.fileName = s.fileName
  }
  return ra, nil
 }
@@ -594,10 +590,9 @@ func (wp *WindowsProbe) parseDeletePathArgs(e *etw.DDEventRecord) (*deletePathAr
  dpa.filePath, _, _, _ = data.ParseUnicodeString(40)
  }
 
- wp.filePathResolverLock.Lock()
- defer wp.filePathResolverLock.Unlock()
- if s, ok := wp.filePathResolver[fileObjectPointer(dpa.fileObject)]; ok {
- dpa.oldPath = s
+ // lru is thread safe, has its own locking
+ if s, ok := wp.filePathResolver.Get(fileObjectPointer(dpa.fileObject)); ok {
+ dpa.oldPath = s.fileName
  // question, should we reset the filePathResolver here?
  }
  return dpa, nil

pkg/security/probe/probe_kernel_file_windows_test.go

@@ -46,14 +46,22 @@ func createTestProbe() (*WindowsProbe, error) {
  if err != nil {
  return nil, err
  }
+ fc, err := lru.New[fileObjectPointer, fileCache](1024)
+ if err != nil {
+ return nil, err
+ }
+ rc, err := lru.New[regObjectPointer, string](1024)
+ if err != nil {
+ return nil, err
+ }
 
  // probe and config are provided as null. During the tests, it is assumed
  // that we will not access those values.
  wp := &WindowsProbe{
  opts: opts,
  config: cfg,
- filePathResolver: make(map[fileObjectPointer]string, 0),
- regPathResolver: make(map[regObjectPointer]string, 0),
+ filePathResolver: fc,
+ regPathResolver: rc,
  discardedPaths: discardedPaths,
  discardedBasenames: discardedBasenames,
  }

pkg/security/probe/probe_kernel_reg_windows.go

@@ -161,10 +161,10 @@ func (wp *WindowsProbe) computeFullPath(cka *createKeyArgs) {
  if strings.HasPrefix(cka.relativeName, regprefix) {
  cka.translateBasePaths()
  cka.computedFullPath = cka.relativeName
- wp.regPathResolver[cka.keyObject] = cka.relativeName
+ wp.regPathResolver.Add(cka.keyObject, cka.relativeName)
  return
  }
- if s, ok := wp.regPathResolver[cka.keyObject]; ok {
+ if s, ok := wp.regPathResolver.Get(cka.keyObject); ok {
  cka.computedFullPath = s
  }
  var outstr string
@@ -175,13 +175,13 @@ func (wp *WindowsProbe) computeFullPath(cka *createKeyArgs) {
  outstr += cka.relativeName
  } else {
 
- if s, ok := wp.regPathResolver[cka.baseObject]; ok {
+ if s, ok := wp.regPathResolver.Get(cka.keyObject); ok {
  outstr = s + "\\" + cka.relativeName
  } else {
  outstr = cka.relativeName
  }
  }
- wp.regPathResolver[cka.keyObject] = outstr
+ wp.regPathResolver.Add(cka.keyObject, outstr)
  cka.computedFullPath = outstr
 }
 func (cka *createKeyArgs) String() string {
@@ -213,7 +213,7 @@ func (wp *WindowsProbe) parseDeleteRegistryKey(e *etw.DDEventRecord) (*deleteKey
  dka.keyObject = regObjectPointer(data.GetUint64(0))
  dka.status = data.GetUint32(8)
  dka.keyName, _, _, _ = data.ParseUnicodeString(12)
- if s, ok := wp.regPathResolver[dka.keyObject]; ok {
+ if s, ok := wp.regPathResolver.Get(dka.keyObject); ok {
  dka.computedFullPath = s
  }
 
@@ -329,7 +329,7 @@ func (wp *WindowsProbe) parseSetValueKey(e *etw.DDEventRecord) (*setValueKeyArgs
 
  sv.previousData = data.Bytes(nextOffset, int(sv.previousDataSize))
 
- if s, ok := wp.regPathResolver[sv.keyObject]; ok {
+ if s, ok := wp.regPathResolver.Get(sv.keyObject); ok {
  sv.computedFullPath = s
  }
 

pkg/security/probe/probe_windows.go

@@ -72,8 +72,8 @@ type WindowsProbe struct {
 
  // path caches
  filePathResolverLock sync.Mutex
- filePathResolver map[fileObjectPointer]string
- regPathResolver map[regObjectPointer]string
+ filePathResolver *lru.Cache[fileObjectPointer, fileCache]
+ regPathResolver *lru.Cache[regObjectPointer, string]
 
  // state tracking
  renamePreArgs renameState
@@ -85,6 +85,11 @@ type WindowsProbe struct {
  discardedBasenames *lru.Cache[string, struct{}]
 }
 
+// filecache currently only has a filename. But this is going to expand really soon. so go ahead
+// and have the wrapper struct even though right now it doesn't add anything.
+type fileCache struct {
+ fileName string
+}
 type renameState struct {
  fileObject uint64
  path string
@@ -301,11 +306,8 @@ func (p *WindowsProbe) setupEtw(ecb etwCallback) error {
  if ca, err := p.parseCloseArgs(e); err == nil {
  log.Tracef("Received Close event %d %s\n", e.EventHeader.EventDescriptor.ID, ca)
  ecb(ca, e.EventHeader.ProcessID)
- if e.EventHeader.EventDescriptor.ID == idClose {
- p.filePathResolverLock.Lock()
- delete(p.filePathResolver, ca.fileObject)
- p.filePathResolverLock.Unlock()
- }
+ // lru is thread safe, has its own locking
+ p.filePathResolver.Remove(ca.fileObject)
  }
  p.stats.fileClose++
  case idFlush:
@@ -398,7 +400,7 @@ func (p *WindowsProbe) setupEtw(ecb etwCallback) error {
  case idRegCloseKey:
  if dka, err := p.parseCloseKeyArgs(e); err == nil {
  log.Tracef("Got idRegCloseKey %s", dka)
- delete(p.regPathResolver, dka.keyObject)
+ p.regPathResolver.Remove(dka.keyObject)
  }
  p.stats.regCloseKey++
  case idQuerySecurityKey:
@@ -695,7 +697,7 @@ func (p *WindowsProbe) Close() error {
 // SendStats sends statistics about the probe to Datadog
 func (p *WindowsProbe) SendStats() error {
  p.filePathResolverLock.Lock()
- fprLen := len(p.filePathResolver)
+ fprLen := p.filePathResolver.Len()
  p.filePathResolverLock.Unlock()
 
  // may need to lock here
@@ -766,7 +768,7 @@ func (p *WindowsProbe) SendStats() error {
  if err := p.statsdClient.Gauge(metrics.MetricWindowsSizeOfFilePathResolver, float64(fprLen), nil, 1); err != nil {
  return err
  }
- if err := p.statsdClient.Gauge(metrics.MetricWindowsSizeOfRegistryPathResolver, float64(len(p.regPathResolver)), nil, 1); err != nil {
+ if err := p.statsdClient.Gauge(metrics.MetricWindowsSizeOfRegistryPathResolver, float64(p.regPathResolver.Len()), nil, 1); err != nil {
  return err
  }
  return nil
@@ -784,6 +786,15 @@ func NewWindowsProbe(probe *Probe, config *config.Config, opts Opts) (*WindowsPr
  return nil, err
  }
 
+ fc, err := lru.New[fileObjectPointer, fileCache](config.RuntimeSecurity.WindowsFilenameCacheSize)
+ if err != nil {
+ return nil, err
+ }
+ rc, err := lru.New[regObjectPointer, string](config.RuntimeSecurity.WindowsRegistryCacheSize)
+ if err != nil {
+ return nil, err
+ }
+
  ctx, cancelFnc := context.WithCancel(context.Background())
 
  p := &WindowsProbe{
@@ -798,8 +809,8 @@ func NewWindowsProbe(probe *Probe, config *config.Config, opts Opts) (*WindowsPr
  onError: make(chan bool),
  onETWNotification: make(chan etwNotification),
 
- filePathResolver: make(map[fileObjectPointer]string, 0),
- regPathResolver: make(map[regObjectPointer]string, 0),
+ filePathResolver: fc,
+ regPathResolver: rc,
 
  discardedPaths: discardedPaths,
  discardedBasenames: discardedBasenames,
@@ -851,7 +862,8 @@ func (p *WindowsProbe) OnNewDiscarder(_ *rules.RuleSet, ev *model.Event, field e
  fileObject := fileObjectPointer(ev.CreateNewFile.File.FileObject)
  p.filePathResolverLock.Lock()
  defer p.filePathResolverLock.Unlock()
- delete(p.filePathResolver, fileObject)
+ //delete(p.filePathResolver, fileObject)
+ p.filePathResolver.Remove(fileObject)
 }
 
 // NewModel returns a new Model