tetragon: Move return filter to kernel #1773
Conversation
olsajiri
added
the
release-note/minor
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
8 times, most recently
from
3dae4f7 to
566fa50
Compare
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
566fa50 to
eb1552a
Compare
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
5 times, most recently
from
4e18f76 to
20416b3
Compare
There was a problem hiding this comment.
Thanks! This looks good overall.
Just some minor comments.
Also: was this #1781 (comment) resolved? Would be nice to reject the tracing policy early if something is not supported.
pkg/bpf/detect.go
Outdated
| prog.Close() | ||
|
|
||
| if err != nil { | ||
| signalHelper.detected = false |
There was a problem hiding this comment.
nit: this seems redundant, since we are setting it based on the return value in the caller.
There was a problem hiding this comment.
true, fixed.. I copy/pasted this from another helper, so I changed that one as well ;-)
| retFilterMap := program.MapBuilderPin("filter_map", sensors.PathJoin(pinPath, "retprobe_filter_map"), loadret) | ||
| maps = append(maps, retFilterMap) | ||
|
|
||
| maps = append(maps, filterMaps(loadret, pinPath, nil)...) |
There was a problem hiding this comment.
Not sure I understand this.
So we append filterMaps(loadret, pinPath, nil) which will always return []*program.Map{}, AFAICT.
func filterMaps(load *program.Program, pinPath string, kprobeEntry *genericKprobe) []*program.Map {
var maps []*program.Map
state := getProgramSelector(load, kprobeEntry)
if state == nil {
return []*program.Map{}
}
And:
func getProgramSelector(load *program.Program, kprobeEntry *genericKprobe) *selectors.KernelSelectorState {
if kprobeEntry != nil {
if load.RetProbe {
return kprobeEntry.loadArgs.selectors.retrn
}
return kprobeEntry.loadArgs.selectors.entry
}
return nil
}
Can't we just remove this line the append here? (And maybe the nil check in the calling functions)?
There was a problem hiding this comment.
right..good catch.. the state is needed for single kprobe where we can fix maximums for kernel < 5.9, so it's not needed for multi kprobe which is available for later kernels, so there's no need for this fixes.. I'll make the fix and add comment
ugh, forgot about this one.. sry, added PR #1863 |
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
20416b3 to
9089df7
Compare
kkourt
added
the
needs-rebase
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
9089df7 to
7ba1d1f
Compare
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
7ba1d1f to
2c84752
Compare
mtardy
removed
the
needs-rebase
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
2c84752 to
7141da5
Compare
Move the prog.Close() to success path and remove superfluous detected bool assignment. Also there's no reason to use defer in detectLargeProgramSize. Signed-off-by: Jiri Olsa <[email protected]>
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
7141da5 to
4be83ca
Compare
|
hum, I think the 32 bit change that got merged blocked this change, because we cross the instructions limit for filter program on small kernels.. we'll need to split the filtering logic and then merge this change |
Move the selector state instance under kprobeSelectors object, because we are adding return selector in following changes so this way both states object are stored together. Signed-off-by: Jiri Olsa <[email protected]>
Adding MatchReturnActions selector data so we can specify actions when return matcher passes. It's allows same actions as MatchAction matcher. Signed-off-by: Jiri Olsa <[email protected]>
Adding InitKernelReturnSelectorState function that creates return probe kernel selector state object. Signed-off-by: Jiri Olsa <[email protected]>
Adding support to create and load return selector into filter maps. Signed-off-by: Jiri Olsa <[email protected]>
Adding tests for return selectors defined: - as empty - with return value matches in 2 int values - with return follow fd action Signed-off-by: Jiri Olsa <[email protected]>
Adding support for filter in return kprobe object. Adding filter and action tail calls to handle return probe filter and possible actions. Signed-off-by: Jiri Olsa <[email protected]>
Now that we have return argument filter support in kernel, we no longer need the user space support, removing it. Due to bpf filter program complexity we can't load GT/LT filters in 4.19 kernels, so I'm disabling GT/LT tests for 4.19 kernels. Let's see if that's a real problem before we get to fun of mixing user and kernel filtering for 4.19 kernels and newer ones. Signed-off-by: Jiri Olsa <[email protected]>
Adding return action field to ProcessKprobe message to pass the return action value to final event. Signed-off-by: Jiri Olsa <[email protected]>
Storing the return action value to final merged kprobe event. Signed-off-by: Jiri Olsa <[email protected]>
Moving TestKprobeSigkill functionality to testSigkill function, so it can be used be following change. Signed-off-by: Jiri Olsa <[email protected]>
Adding sigkill return action test that is triggered by return lseek kprobe. Signed-off-by: Jiri Olsa <[email protected]>
Updating the selectors docs with return actions info. Signed-off-by: Jiri Olsa <[email protected]>
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
4be83ca to
a8bde06
Compare
Some of the killer tests are missing signal helper checks, adding them. Signed-off-by: Jiri Olsa <[email protected]>
olsajiri
force-pushed
the
pr/olsajiri/retkprobe_filter
branch
from
2990435 to
8a07d09
Compare
ok, managed to reorg the code a bit and we're still good ;-) but I'll start on the filter re-work anyway |
moving the return filter to bpf program and adding support for return actions