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

new: update SDK to plugin API version 3.6.0 #38

Merged
merged 4 commits into from
Jun 26, 2024

Conversation

mrgian
Copy link
Contributor

@mrgian mrgian commented Jun 17, 2024

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area plugin-sdk

/area tests

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Copy link
Contributor

@jasondellaluce jasondellaluce left a comment

Choose a reason for hiding this comment

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

Minor points to be solved before merging

examples/syscall_parse/plugin.cpp Outdated Show resolved Hide resolved
examples/syscall_parse/plugin.cpp Outdated Show resolved Hide resolved
examples/syscall_extract/plugin.cpp Outdated Show resolved Hide resolved
examples/syscall_parse/plugin.cpp Outdated Show resolved Hide resolved
include/falcosecurity/internal/plugin_mixin_common.h Outdated Show resolved Hide resolved
@incertum
Copy link

Tagging myself here to know when this gets merged.

Signed-off-by: Gianmatteo Palmieri <[email protected]>
@mrgian
Copy link
Contributor Author

mrgian commented Jun 24, 2024

Minor points to be solved before merging

I solved these points but please don't merge this for now.

I've tried running this example for a while https://github.com/mrgian/plugin-sdk-cpp/blob/update-3.6.0-new/examples/syscall_parse/plugin.cpp
And it looks like repeated subtable writes are causing a segfault.
I'm investigating to see if the issue lies in the libs or the SDK 👀

mrgian and others added 2 commits June 26, 2024 10:26
Signed-off-by: Gianmatteo Palmieri <[email protected]>
Co-authored-by: Jason Dellaluce <[email protected]>
Signed-off-by: Gianmatteo Palmieri <[email protected]>
@mrgian
Copy link
Contributor Author

mrgian commented Jun 26, 2024

And it looks like repeated subtable writes are causing a segfault.
I'm investigating to see if the issue lies in the libs or the SDK 👀

After investigating the issue with @jasondellaluce, looks like the the issue is on the libs side.
This PR should be ready to be merged :)

@incertum I also added a new example showing how to access the args and the fd_table from the event TID.
If the example is not clear enough please let me know :)

@jasondellaluce the example has a lot of prints, should we keep it as is or should we remove them before merging?

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link

poiana commented Jun 26, 2024

LGTM label has been added.

Git tree hash: 4e71d0d6997a520e57ebf14865b3b6a5bed3249b

@poiana
Copy link

poiana commented Jun 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, mrgian

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit 1c46ba0 into falcosecurity:master Jun 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants