Add dynamic extraction of a parameter attribute #3143
+776
−14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
from
9a9d21f to
ce33ec7
Compare
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
from
ce33ec7 to
5ebb3e0
Compare
2 tasks
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
2 times, most recently
from
e3117d7 to
ba048b3
Compare
olsajiri
reviewed
Nov 22, 2024
There was a problem hiding this comment.
I like it, left some comments
thanks for splitting the code in multiple logical changes, it's rare ;-)
we were originally thinking of using C expression parsing (there's some go lib for that) but I think that can be done later if it's ever needed, the basic parsing you did should be fine
please add tests, would be great to have some framework where we could easily add various 'ExtractParam' expressions for testing
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
from
ba048b3 to
1fb7dbc
Compare
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
2 times, most recently
from
753fd10 to
2b1153f
Compare
ScriptSathi
changed the title
This commit introduces the `struct config_btf_arg_depth`. It appends
`btf_argN` to `struct event_config` as an array. This array stores the
path to the searched data.
Any `btf_argN` can have a list of elements as follow :
file->f_path is 152 bytes, so the array will look like
[{ offset: 152, is_pointer: 0, is_initialized: 1 }, ...].
The max value `MAX_BTF_ARG_DEPTH` as been set arbitrary as the verifier
need a fixed size.
In config_btf_arg, is_pointer and is_initialized are u16 because it must
match padding of 64 bits long structure
Signed-off-by: Tristan d'Audibert <[email protected]>
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
from
2b1153f to
a04e634
Compare
added 2 commits
January 10, 2025 17:54
…_argN This commit introduces the “extract_arg_depth” function and loops over it to move into the “arg” buffer of config->btf_argN[i]->offset by iteration. The goal is to reach the requiered data by overwriting over arg with the new value. Signed-off-by: Tristan d'Audibert <[email protected]>
This commit checks if btf.Type exists in Tetragon existing types. For instance: `struct file` with btf is called `file` and also exists in `GenericStringToType` with the same alias. However, the attribute `name` in `struct qstr` has a returned type `unsigned char` wich does not exist yet in `GenericStringToType`. The same thing happened with `linux_binprm->filename` as the type is `char` Signed-off-by: Tristan d'Audibert <[email protected]>
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
from
a04e634 to
058c828
Compare
added 7 commits
January 10, 2025 19:11
FindNextBtfType function recursively searches in a btf structure in order to find a specific path until it reaches the target or fails. The function also searches in embedded anonymous structures or unions to cover as much use cases as possible. For instance, mm_struct has 2 fields; anonymous struct and another type. But you are still able to look into the anonymous struct by specifying a path like "mm.pgd.pgd". For instance, if the search is in the linux_binprm structure and the path is `file.f_path.dentry.d_name.name`, the following actions will be done. - Look for the variable name `file` inside `linux_binprm`. - If it matches, it stores the offset from linux_binprm where the `file` variable could be found. - Then it takes the btf type `file` and searches for a parameter named `f_path`. Signed-off-by: Tristan d'Audibert <[email protected]>
In order to read the data properly on BPF side, integer/long values must use `bpf_probe_read`. So now, every time the latest type retrieved is defined as an integer, it will be safely read before accessing the data. Signed-off-by: Tristan d'Audibert <[email protected]>
This commit adds 2 parameters to give the ability to the user to search
for a specific variable following a "path" as follow:
```yml
...
args:
- index: 0
type: "linux_binprm"
extractParam: "file.f_path.dentry.d_name.name"
overwriteType: "string"
...
```
The above config can be used to extract a specific parameter from the
structure at index 0.
Signed-off-by: Tristan d'Audibert <[email protected]>
The OverwriteType parameter should be deleted if `argSelectorType` can use the `EventConfig` structure to search for the correct Type. Signed-off-by: Tristan d'Audibert <[email protected]>
Searches if every user defined type with ExtractParam exists as a BTF
type and stores its corresponding offset in ConfigBtfArg.
This function does a basic split on `ExtractParam` to obtain the "path"
to the required data. Then, the array is given to `btf.FindNextBTFType` to
find the offset of each element until we reach the required data.
The output is stored in EventConfig to keep the normal behaviour.
For example, if the arg 0 is `struct linux_binprm` and ExtractParam is
set to `file.f_path.dentry.d_name.name`, the output will give an array
of all the offsets from their parents as follows :
[{ offset: 96, is_pointer: 0 }, { offset: 152, is_pointer: 1 }, ...]
Signed-off-by: Tristan d'Audibert <[email protected]>
This commit updates `addLsm` function to use the `ExtractParam` and `OverwriteType` in order to look for the attributes in BTF structure. Signed-off-by: Tristan d'Audibert <[email protected]>
…probes As BTF types are not defined for Uprobes, their offsets can't be found in BTF file. Thus, with this commit, if the user defines ExtractParam / OverwriteType, their are ignored and a warning is displayed Signed-off-by: Tristan d'Audibert <[email protected]>
added 5 commits
January 10, 2025 19:11
Add very similar code as in `genericlsm.go` file to handle ExtractParam feature. Signed-off-by: Tristan d'Audibert <[email protected]>
This commit adds 3 tests for FindNextBtfType algorithm. The first is `testAssertEqualBtfPath` to assert that a specific path has the exact same btfConfig as expected. The second, "testAssertPathIsAccessible" tries to reach the path and asserts that no errors are raised. The third test "testAssertErrorOnInvalidPath" asserts that the error messages raised if the path is incorrect. The chosen test cases have embed union/anonymous structs. The important thing to notice in case of adding new tests is to be aware of btf changes on different architectures. Especially for `testAssertEqualBtfPath` where Offset could be different. To Test locally : ``` go test -exec "sudo" ./pkg/btf/ ``` Signed-off-by: Tristan d'Audibert <[email protected]>
This test aims at testing the ExtractParam feature. Tetragon use hard-coded types instead of BTF. So currently, the ExtractParam feature does not allow to extract attributes from other types than the few hard-coded in `generictypes.go`. For instance, if you try to use the feature with task_struct structure, it will fail because it is not yet hard-coded. Signed-off-by: Tristan d'Audibert <[email protected]>
Signed-off-by: Tristan d'Audibert <[email protected]>
Signed-off-by: Tristan d'Audibert <[email protected]>
ScriptSathi
force-pushed
the
pr/ScriptSathi/add-dynamic-paramter-extraction
branch
from
058c828 to
391c5a8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The discussion for this PR can be found here #3142
This is currently a draft.
I wanted to start the discussion before submitting the final code, as I think, it is a big enough PR.
Take this PR in the today state as a proof of concept for dynamic parameter extraction. I will continue to work on this PR until the below checks are done.
At the current state, the PR is able to
Description
This PR introduce the dynamic extraction of a custom attribute
Comments
OverwriteTypeparameter. But since the functionargSelectorTypedoes not receiveEventConfig, it is not possible to search for the type using directly BTF types. So I suggest doing another PR before this one is merged to do so if possible. Then I'll remove the parameter. It is also not possible to add an if condition foruprobesin this function. So if the user defines the parameter, it will overwrite the type at this moment.u8to store the offset, as the verifier does not allow me to useu16. At this moment, I don't know if it is possible to found offset > 255 in BTF structures. For my uses cases, usingu8was enough.Test the PR
You can use the following config
If you want to test it with more arguments, you can use
bprm_creds_from_filehook. It hasstruct linux_binprmandstruct filewhich are supported.Release-note