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

tetragon: Add support for uprobe arguments #1978

Merged
merged 14 commits into from
Jan 25, 2024
Merged

Conversation

olsajiri
Copy link
Contributor

@olsajiri olsajiri commented Jan 15, 2024

Adding support for argument values in uprobes and factor the code a bit so we could shre the arguments code with kprobes.

Copy link

netlify bot commented Jan 15, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 81c9149
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/65ad8c1e1e81da0008783de5
😎 Deploy Preview https://deploy-preview-1978--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@olsajiri olsajiri force-pushed the pr/olsajiri/uprobe_args branch from ff1b59a to dad139a Compare January 17, 2024 12:58
@olsajiri olsajiri changed the title Pr/olsajiri/uprobe args tetragon: Add support for uprobe arguments Jan 17, 2024
@olsajiri olsajiri added the release-note/minor This PR introduces a minor user-visible change label Jan 17, 2024
@olsajiri olsajiri force-pushed the pr/olsajiri/uprobe_args branch 10 times, most recently from a3ce6b1 to 733a127 Compare January 22, 2024 07:19
@olsajiri olsajiri marked this pull request as ready for review January 22, 2024 08:04
@olsajiri olsajiri requested review from a team and mtardy as code owners January 22, 2024 08:04
@olsajiri olsajiri requested review from kkourt and jrfastab and removed request for kkourt January 22, 2024 08:04
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think it would be nice to get another review as I mostly read pretty quickly the PR commit by commit but maybe missed stuff. But the tests and code look good.

But again, I wish I could write PR with commits clearly separated like that, I'm learning from this!

For a follow-up PR it would be good to improve the doc example by retrieving the bash readline argument to show that you can eavesdrop on someone's bash session now that it's possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool thanks for adding a kprobe args test!

Renaming argPrinters type to argPrinter which makes
more sense to me.

Signed-off-by: Jiri Olsa <[email protected]>
Moving argument helpers code to args.go to structure the code better,
because it will be used from uprobe code in following changes.

Signed-off-by: Jiri Olsa <[email protected]>
Moving argument processing code to args.go structure the code better,
because it will be used also from uprobe code. Moving it together with
two other helper functions.

Signed-off-by: Jiri Olsa <[email protected]>
Moving argument grpc code to getKprobeArgument function,
so it can be used from uprobe code in following changes.

Signed-off-by: Jiri Olsa <[email protected]>
Adding signed long kprobe argument type, because it's missing
and it will be used in uprobe test spec.

We already have long_arg in KprobeArgument message and the support
in spec and generictypes, we're just missing the processing code.

Signed-off-by: Jiri Olsa <[email protected]>
At the moment the smallest value we can read as argument value on
bpf side is 32-bit, which might be tricky if rest of the register
carrying the argument value is not zeroed out.

Adding support to specify 8 and 16 bit values as argument types in
the spec and read just the relevant portion of the data.

Signed-off-by: Jiri Olsa <[email protected]>
Adding more types to btf compatibility validation after adding
and using several new types in previous changes.

Signed-off-by: Jiri Olsa <[email protected]>
Adding args array to uprobe spec. Using the KProbeArg object
for that, so we can re-use all the arguments code.

Signed-off-by: Jiri Olsa <[email protected]>
Adding args field to ProcessUprobe message to carry out
the uprobe's arguments. Using the KprobeArgument object
for that, so we can re-use all the arguments code.

Signed-off-by: Jiri Olsa <[email protected]>
Adding support to process uprobe arguments the same way
we do that for kprobes, so we can reuse the code.

Using the argPrinter objects to register argument values
for uprobe and the decode their values for each event.

Adding arguments retrieval code into bpf uprobe event
setup code.

Signed-off-by: Jiri Olsa <[email protected]>
Adding uprobe test functions for argument values that
will be used in following test code.

Signed-off-by: Jiri Olsa <[email protected]>
Adding test for uprobe arguments retrieval from
all uprobe_test_lib_arg* functions.

Signed-off-by: Jiri Olsa <[email protected]>
Adding kprobe args test by hooking bpf_fentry_test[1-5] functions,
which have all needed argument types for testing.

Signed-off-by: Jiri Olsa <[email protected]>
We now have multiple symbols, not just one.

The rest of the documentation wrt argument processing is now
correct due to the previous patches.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri olsajiri force-pushed the pr/olsajiri/uprobe_args branch from 733a127 to dd77b6f Compare January 23, 2024 08:28
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

bpf/process/types/basic.h Show resolved Hide resolved
@olsajiri olsajiri merged commit 6aba354 into main Jan 25, 2024
38 checks passed
@olsajiri olsajiri deleted the pr/olsajiri/uprobe_args branch January 25, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants