Skip to content

Commit

Permalink
server: make a copy of exit messages when filtering fields
Browse files Browse the repository at this point in the history
We encountered another segfault similar to the one fixed in #1432. This time, it's related
to the pid field in exit events. Fix this by making a copy of the exit event before
filtering. Also optimize copying logic to only perform the copy when absolutely necessary.

Signed-off-by: William Findlay <will@isovalent.com>
  • Loading branch information
willfindlay committed Nov 1, 2023
1 parent 962f0d7 commit 7344eac
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 7 deletions.
43 changes: 43 additions & 0 deletions pkg/filters/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,51 @@ type FieldFilter struct {
fields fmutils.NestedMask
action tetragon.FieldFilterAction
invertEventSet bool
needsCopy map[tetragon.EventType]struct{}
}

// NewFieldFilter constructs a new FieldFilter from a set of fields.
func NewFieldFilter(eventSet []tetragon.EventType, fields []string, action tetragon.FieldFilterAction, invertEventSet bool) *FieldFilter {
var maybeNeedsCopy bool
if action == tetragon.FieldFilterAction_EXCLUDE {
for _, field := range fields {
if field == "process.pid" || field == "parent.pid" {
maybeNeedsCopy = true
break
}
}
} else if action == tetragon.FieldFilterAction_INCLUDE {
var hasPid, hasPpid bool
maybeNeedsCopy = true
for _, field := range fields {
if field == "process.pid" {
hasPid = true
}
if field == "parent.pid" {
hasPpid = true
}
}
if hasPid && hasPpid {
maybeNeedsCopy = false
}
}

needsCopy := make(map[tetragon.EventType]struct{})
for _, t := range eventSet {
if t == tetragon.EventType_PROCESS_EXEC && maybeNeedsCopy {
needsCopy[tetragon.EventType_PROCESS_EXEC] = struct{}{}
}
if t == tetragon.EventType_PROCESS_EXIT && maybeNeedsCopy {
needsCopy[tetragon.EventType_PROCESS_EXIT] = struct{}{}
}
}

return &FieldFilter{
eventSet: eventSet,
fields: fmutils.NestedMaskFromPaths(fields),
action: action,
invertEventSet: invertEventSet,
needsCopy: needsCopy,
}
}

Expand Down Expand Up @@ -141,6 +177,13 @@ func FieldFiltersFromGetEventsRequest(request *tetragon.GetEventsRequest) []*Fie
return filters
}

func (f *FieldFilter) NeedsCopy(ev *tetragon.GetEventsResponse) bool {
if _, ok := f.needsCopy[ev.EventType()]; ok {
return true
}
return false
}

// Filter filters the fields in the GetEventsResponse, keeping fields specified in the
// inclusion filter and discarding fields specified in the exclusion filter. Exclusion
// takes precedence over inclusion and an empty filter set will keep all remaining fields.
Expand Down
14 changes: 7 additions & 7 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,14 @@ func (s *Server) GetEventsWG(request *tetragon.GetEventsRequest, server tetragon
// Filter the GetEventsResponse fields
filters := filters.FieldFiltersFromGetEventsRequest(request)
filterEvent := event
if len(filters) > 0 && filterEvent.GetProcessExec() != nil { // this is an exec event and we have fieldFilters
// We need a copy of the exec event as modifing the original message
// can cause issues in the process cache (we keep a copy of that message there).
filterEvent = proto.Clone(event).(*tetragon.GetEventsResponse)
}

var copied bool
for _, filter := range filters {
// we need not to change res
// maybe only for exec events
// We need a copy of the exec or exit event as modifing the original message
// can cause issues in the process cache (we keep a copy of that message there).
if filter.NeedsCopy(filterEvent) && !copied {
filterEvent = proto.Clone(event).(*tetragon.GetEventsResponse)
}
filter.Filter(filterEvent)
}

Expand Down

0 comments on commit 7344eac

Please sign in to comment.