libs icon indicating copy to clipboard operation
libs copied to clipboard

wip: chore/improvements

Open FedeDP opened this issue 3 years ago • 13 comments

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 driver-kmod

/area driver-ebpf

/area libscap

/area libsinsp

/area tests

/area proposals

What this PR does / why we need it:

This PR has been splitted from #86.

  • added a flexible array member to struct ppm_evt_hdr to manage events payload Std states:

As a special case, the last element of a structure with more than one named member may have an incomplete array type; this is called a flexible array member. In most situations, the flexible array member is ignored. In particular, the size of the structure is as if the flexible array member were omitted except that it may have more trailing padding than the omission would imply.

Note about the padding: we use an uint8_t for flexible array member, thus it shall not change padding in our case: you store sizeof(ppm_evt_hdr) + sizeof hdrs + sizeof(payload) and you load exactly the same; but the flexible array member helps in avoiding pointer arithmetic when dealing with hdrs and payload.

I tested the dump/read of scap files:

  1. new version from new version
  2. new version from old version
  3. old version from new version
  • merged g_syscall_code_routing_table into g_syscall_table The g_syscall_code_routing_table was not really useful by itself, and the 2 tables had same indexing.

  • dropped syscall_info_table; it is now automatically generated during first call to scap_get_syscall_info_table using data from syscall_table and event_table. This way, categories or flags cannot be desynced anymore.

  • forced gnu99 as C standard

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

FedeDP avatar Apr 22 '22 14:04 FedeDP

LGTM label has been added.

Git tree hash: 67d6ead259b50e92e94a915b3c737d8e84bbb5de

poiana avatar Apr 24 '22 18:04 poiana

LGTM label has been added.

Git tree hash: 5fa4cf3bd8ca592ca229cbc30ec8fe5084dd302a

poiana avatar Apr 26 '22 08:04 poiana

[APPROVALNOTIFIER] This PR is APPROVED

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

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:
  • ~~OWNERS~~ [Andreagit97,FedeDP]

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

poiana avatar Apr 26 '22 08:04 poiana

Hold this one until Falco 0.32.0 is out; too big and risky change (even if it is just a big refactor).

/hold

FedeDP avatar Apr 26 '22 12:04 FedeDP

/test build-libs

FedeDP avatar Apr 28 '22 14:04 FedeDP

Remainder for myself: understand why syscall_info_table has EF_NONE/EF_DROP_SIMPLE_CONS (ie: a subset of event_table event flags). Can't we drop the flags from there and deduplicate a bit these 2 tables?

FedeDP avatar May 24 '22 13:05 FedeDP

/cc @Andreagit97 @alacuku

FedeDP avatar May 24 '22 13:05 FedeDP

@FedeDP: GitHub didn't allow me to request PR reviews from the following users: alacuku.

Note that only falcosecurity members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @Andreagit97 @alacuku

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

poiana avatar May 24 '22 13:05 poiana

New changes are detected. LGTM label has been removed.

poiana avatar May 25 '22 09:05 poiana

TODO for completely dropping syscall_info_table, in favor of an autogenerated table during first call to scap_get_syscall_info_table:

  • fix category and name for syscalls that are not mapped to a specific event (like PPM_SC_EXIT and PPM_SC_RESTART_SYSCALL)

FedeDP avatar May 25 '22 09:05 FedeDP

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana avatar Aug 28 '22 09:08 poiana

Stale issues rot after 30d of inactivity.

Mark the issue as fresh with /remove-lifecycle rotten.

Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle rotten

poiana avatar Sep 27 '22 09:09 poiana

Biggest parts of this one, ie:

merged g_syscall_code_routing_table into g_syscall_table The g_syscall_code_routing_table was not really useful by itself, and the 2 tables had same indexing.

Merged.

dropped syscall_info_table; it is now automatically generated during first call to scap_get_syscall_info_table using data from syscall_table and event_table. This way, categories or flags cannot be desynced anymore.

There is an open PR for that: #649

The only remaining part depends upon C99. I am not sure whether it is an issue or now. Still, i think the flexible array member cleanup is good from a dev PoV.

FedeDP avatar Oct 05 '22 15:10 FedeDP

Rotten issues close after 30d of inactivity.

Reopen the issue with /reopen.

Mark the issue as fresh with /remove-lifecycle rotten.

Provide feedback via https://github.com/falcosecurity/community. /close

poiana avatar Nov 04 '22 15:11 poiana

@poiana: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue with /reopen.

Mark the issue as fresh with /remove-lifecycle rotten.

Provide feedback via https://github.com/falcosecurity/community. /close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

poiana avatar Nov 04 '22 15:11 poiana