Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Memory leak in diff plugin #3555

Merged
merged 5 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Bugfix: Memory leak in diff plugin
The diff plugin did not run each iteration in an isolated scope so
cleanup destructors where not being called.

Additionally registry cache was not installed in the root scope which
limited its usefulness in sub queries mostly leading to cache misses.

The default behaviour of TTL cache is to extend the ttl on each Get()
operation. This is hardly ever what we want because then some cached
items will never expire.
  • Loading branch information
scudette committed Jun 11, 2024
commit da2ae28e335afb4cb968c8e9c3ba31506936ee66
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ windows_bare:
windowsx86:
go run make.go -v windowsx86

windowsarm:
go run make.go -v windowsarm

clean:
go run make.go -v clean

Expand Down
5 changes: 0 additions & 5 deletions accessors/ext4/ext4_accessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ import (
"www.velocidex.com/golang/vfilter"
)

const (
// Scope cache tag for the FAT parser
Ext4FileSystemTag = "_EXT4"
)

type Ext4FileInfo struct {
*ext4.FileInfo
_full_path *accessors.OSPath
Expand Down
2 changes: 2 additions & 0 deletions accessors/process/process_address_space_windows.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build windows && amd64 && cgo
// +build windows,amd64,cgo

// An accessor for process address space.
Expand Down Expand Up @@ -233,6 +234,7 @@ func (self ProcessAccessor) New(scope vfilter.Scope) (
scope: scope,
}
result.lru.SetTTL(time.Second)
result.lru.SkipTTLExtensionOnHit(true)
result.lru.SetCheckExpirationCallback(func(key string, value interface{}) bool {
info, ok := value.(*ProcessReader)
if ok {
Expand Down
68 changes: 68 additions & 0 deletions accessors/raw_registry/cache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package raw_registry

import (
"time"

"github.com/Velocidex/ttlcache/v2"
"www.velocidex.com/golang/velociraptor/constants"
vql_subsystem "www.velocidex.com/golang/velociraptor/vql"
"www.velocidex.com/golang/vfilter"
)

const (
RAW_CACHE_TAG = "_RAW_REG_CACHE"
)

type RawRegFileSystemAccessorCache struct {
lru *ttlcache.Cache
readdir_lru *ttlcache.Cache
}

func (self *RawRegFileSystemAccessorCache) Close() {
self.lru.Close()
self.readdir_lru.Close()
}

func getRegFileSystemAccessorCache(scope vfilter.Scope) *RawRegFileSystemAccessorCache {
cache, ok := vql_subsystem.CacheGet(
scope, RAW_CACHE_TAG).(*RawRegFileSystemAccessorCache)
if ok {
return cache
}

cache = &RawRegFileSystemAccessorCache{
lru: ttlcache.NewCache(),
readdir_lru: ttlcache.NewCache(),
}

cache_size := int(vql_subsystem.GetIntFromRow(
scope, scope, constants.RAW_REG_CACHE_SIZE))
if cache_size == 0 {
cache_size = 1000
}

cache_time := vql_subsystem.GetIntFromRow(
scope, scope, constants.RAW_REG_CACHE_TIME)
if cache_time == 0 {
cache_time = 10
}

cache.lru.SetCacheSizeLimit(cache_size)
cache.lru.SetTTL(time.Second * time.Duration(cache_time))
cache.lru.SkipTTLExtensionOnHit(true)

cache.readdir_lru.SetCacheSizeLimit(cache_size)
cache.readdir_lru.SetTTL(time.Second * time.Duration(cache_time))
cache.readdir_lru.SkipTTLExtensionOnHit(true)

// Add the cache to the root scope so it can be visible outside
// our scope. This should maximize cache hits
root_scope := vql_subsystem.GetRootScope(scope)

root_scope.AddDestructor(func() {
cache.Close()
})
vql_subsystem.CacheSet(root_scope, RAW_CACHE_TAG, cache)

return cache
}
32 changes: 5 additions & 27 deletions accessors/raw_registry/raw_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"time"

"github.com/Velocidex/ordereddict"
"github.com/Velocidex/ttlcache/v2"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"

Expand Down Expand Up @@ -261,8 +260,7 @@ type RawRegFileSystemAccessor struct {
scope vfilter.Scope
root *accessors.OSPath

lru *ttlcache.Cache
readdir_lru *ttlcache.Cache
cache *RawRegFileSystemAccessorCache
}

func getRegHiveCache(scope vfilter.Scope) *rawHiveCache {
Expand Down Expand Up @@ -310,7 +308,7 @@ func getRegHive(scope vfilter.Scope,
paged_reader, err := readers.NewAccessorReader(
scope, pathspec.DelegateAccessor, delegate, int(lru_size))
if err != nil {
scope.Log("%v: did you provide a URL or Pathspec?", err)
scope.Log("%v: did you provide a Pathspec?", err)
return nil, err
}

Expand Down Expand Up @@ -339,30 +337,10 @@ const RawRegFileSystemTag = "_RawReg"
func (self *RawRegFileSystemAccessor) New(scope vfilter.Scope) (
accessors.FileSystemAccessor, error) {

my_lru := self.lru
if my_lru == nil {
my_lru = ttlcache.NewCache()
my_lru.SetCacheSizeLimit(1000)
my_lru.SetTTL(time.Minute)
}

my_readdir_lru := self.readdir_lru
if my_readdir_lru == nil {
my_readdir_lru = ttlcache.NewCache()
my_readdir_lru.SetCacheSizeLimit(1000)
my_readdir_lru.SetTTL(time.Minute)
}
scope.AddDestructor(func() {
my_lru.Close()
my_readdir_lru.Close()
})

return &RawRegFileSystemAccessor{
scope: scope,
root: self.root,

lru: my_lru,
readdir_lru: my_readdir_lru,
cache: getRegFileSystemAccessorCache(scope),
}, nil
}

Expand Down Expand Up @@ -467,7 +445,7 @@ func (self *RawRegFileSystemAccessor) _readDirWithOSPath(
full_path *accessors.OSPath) (result []accessors.FileInfo, key *regparser.CM_KEY_NODE, err error) {

cache_key := full_path.String()
cached, err := self.readdir_lru.Get(cache_key)
cached, err := self.cache.readdir_lru.Get(cache_key)
if err == nil {
cached_res, ok := cached.(*readDirLRUItem)
if ok {
Expand All @@ -479,7 +457,7 @@ func (self *RawRegFileSystemAccessor) _readDirWithOSPath(

// Cache the result of this function
defer func() {
self.readdir_lru.Set(cache_key, &readDirLRUItem{
self.cache.readdir_lru.Set(cache_key, &readDirLRUItem{
children: result,
err: err,
key: key,
Expand Down
67 changes: 67 additions & 0 deletions accessors/registry/cache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package registry

import (
"time"

"github.com/Velocidex/ttlcache/v2"
"www.velocidex.com/golang/velociraptor/constants"
vql_subsystem "www.velocidex.com/golang/velociraptor/vql"
"www.velocidex.com/golang/vfilter"
)

const (
CACHE_TAG = "_REG_CACHE"
)

type RegFileSystemAccessorCache struct {
lru *ttlcache.Cache
readdir_lru *ttlcache.Cache
}

func (self *RegFileSystemAccessorCache) Close() {
self.lru.Close()
self.readdir_lru.Close()
}

func getRegFileSystemAccessorCache(scope vfilter.Scope) *RegFileSystemAccessorCache {
cache, ok := vql_subsystem.CacheGet(scope, CACHE_TAG).(*RegFileSystemAccessorCache)
if ok {
return cache
}

cache = &RegFileSystemAccessorCache{
lru: ttlcache.NewCache(),
readdir_lru: ttlcache.NewCache(),
}

cache_size := int(vql_subsystem.GetIntFromRow(
scope, scope, constants.REG_CACHE_SIZE))
if cache_size == 0 {
cache_size = 1000
}

cache_time := vql_subsystem.GetIntFromRow(
scope, scope, constants.REG_CACHE_TIME)
if cache_time == 0 {
cache_time = 10
}

cache.lru.SetCacheSizeLimit(cache_size)
cache.lru.SetTTL(time.Second * time.Duration(cache_time))
cache.lru.SkipTTLExtensionOnHit(true)

cache.readdir_lru.SetCacheSizeLimit(cache_size)
cache.readdir_lru.SetTTL(time.Second * time.Duration(cache_time))
cache.readdir_lru.SkipTTLExtensionOnHit(true)

// Add the cache to the root scope so it can be visible outside
// our scope. This should maximize cache hits
root_scope := vql_subsystem.GetRootScope(scope)

root_scope.AddDestructor(func() {
cache.Close()
})
vql_subsystem.CacheSet(root_scope, CACHE_TAG, cache)

return cache
}
7 changes: 6 additions & 1 deletion accessors/registry/lru.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@

package registry

import "www.velocidex.com/golang/velociraptor/accessors"
import (
"time"

"www.velocidex.com/golang/velociraptor/accessors"
)

type readDirLRUItem struct {
children []accessors.FileInfo
err error
age time.Time
}
34 changes: 8 additions & 26 deletions accessors/registry/registry_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ import (
"time"

"github.com/Velocidex/ordereddict"
"github.com/Velocidex/ttlcache/v2"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"

"golang.org/x/sys/windows/registry"
"www.velocidex.com/golang/velociraptor/accessors"
"www.velocidex.com/golang/velociraptor/json"
"www.velocidex.com/golang/velociraptor/utils"
"www.velocidex.com/golang/vfilter"
)

Expand Down Expand Up @@ -373,33 +373,14 @@ func NewValueBuffer(buf []byte, stat accessors.FileInfo) *ValueBuffer {
}

type RegFileSystemAccessor struct {
lru *ttlcache.Cache
readdir_lru *ttlcache.Cache
cache *RegFileSystemAccessorCache
}

func (self *RegFileSystemAccessor) New(scope vfilter.Scope) (
accessors.FileSystemAccessor, error) {
my_lru := self.lru
if my_lru == nil {
my_lru = ttlcache.NewCache()
my_lru.SetCacheSizeLimit(1000)
my_lru.SetTTL(time.Minute)
}

my_readdir_lru := self.readdir_lru
if my_readdir_lru == nil {
my_readdir_lru = ttlcache.NewCache()
my_readdir_lru.SetCacheSizeLimit(1000)
my_readdir_lru.SetTTL(time.Minute)
}
scope.AddDestructor(func() {
my_lru.Close()
my_readdir_lru.Close()
})

return &RegFileSystemAccessor{
lru: my_lru,
readdir_lru: my_readdir_lru,
cache: getRegFileSystemAccessorCache(scope),
}, nil
}

Expand All @@ -423,7 +404,7 @@ func (self RegFileSystemAccessor) ReadDirWithOSPath(
full_path *accessors.OSPath) (result []accessors.FileInfo, err error) {

cache_key := full_path.String()
cached, err := self.readdir_lru.Get(cache_key)
cached, err := self.cache.readdir_lru.Get(cache_key)
if err == nil {
cached_res, ok := cached.(*readDirLRUItem)
if ok {
Expand All @@ -435,9 +416,10 @@ func (self RegFileSystemAccessor) ReadDirWithOSPath(

// Cache the result of this function
defer func() {
self.readdir_lru.Set(cache_key, &readDirLRUItem{
self.cache.readdir_lru.Set(cache_key, &readDirLRUItem{
children: result,
err: err,
age: utils.GetTime().Now(),
})
}()

Expand Down Expand Up @@ -644,7 +626,7 @@ func (self *RegFileSystemAccessor) LstatWithOSPath(
func (self *RegFileSystemAccessor) getCachedKeyInfo(full_path *accessors.OSPath) (
*RegKeyInfo, error) {
cache_key := full_path.String()
cached, err := self.lru.Get(cache_key)
cached, err := self.cache.lru.Get(cache_key)
if err == nil {
res, ok := cached.(*RegKeyInfo)
if ok {
Expand Down Expand Up @@ -673,7 +655,7 @@ func (self *RegFileSystemAccessor) buildAndCacheKeyInfo(
}

cache_key := full_path.String()
self.lru.Set(cache_key, res)
self.cache.lru.Set(cache_key, res)
return res, nil
}

Expand Down
1 change: 1 addition & 0 deletions api/static.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func NewCachedFilesystem(fs http.FileSystem) *CachedFilesystem {
}

result.lru.SetTTL(10 * time.Minute)
result.lru.SkipTTLExtensionOnHit(true)
return result
}

Expand Down
5 changes: 5 additions & 0 deletions constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ const (
// Max age of VSS in (int) days we will consider.
VSS_MAX_AGE_DAYS = "VSS_MAX_AGE_DAYS"

// Controls the lifetime of the registry cache.
REG_CACHE_SIZE = "REG_CACHE_SIZE"
REG_CACHE_TIME = "REG_CACHE_TIME"

RAW_REG_CACHE_SIZE = "RAW_REG_CACHE_SIZE"
RAW_REG_CACHE_TIME = "RAW_REG_CACHE_TIME"
BINARY_CACHE_SIZE = "BINARY_CACHE_SIZE"
EVTX_FREQUENCY = "EVTX_FREQUENCY"
USN_FREQUENCY = "USN_FREQUENCY"
Expand Down
7 changes: 5 additions & 2 deletions crypto/client/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,15 @@ func NewCryptoManager(config_obj *config_proto.Config,
}

result.unauthenticated_lru.SetTTL(time.Second * 60)
result.unauthenticated_lru.SkipTTLExtensionOnHit(true)

return result, nil
}

/* Verify the HMAC protecting the cipher properties blob.
/*
Verify the HMAC protecting the cipher properties blob.

The HMAC ensures that the cipher properties can not be modified.
The HMAC ensures that the cipher properties can not be modified.
*/
func CalcHMAC(
comms *crypto_proto.ClientCommunication,
Expand Down
Loading
Loading