Skip to content

Support execve exit events and clone child exit events on ARM64#416

Merged
poiana merged 18 commits intomasterfrom
fix_arm_registers
Jun 30, 2022
Merged

Support execve exit events and clone child exit events on ARM64#416
poiana merged 18 commits intomasterfrom
fix_arm_registers

Conversation

@Andreagit97
Copy link
Copy Markdown
Member

@Andreagit97 Andreagit97 commented Jun 21, 2022 •
edited
Loading

What type of PR is this?

/kind bug

/kind feature

Any specific area of the project related to this PR?

/area driver-kmod

/area driver-bpf

/area libscap-engine-bpf

What this PR does / why we need it:

This PR wants to fix a known problem regarding ARM64 architecture, described here in this thread of the kernel mailing list. In a nutshell the exeve exit event is not traced by the syscalls:sys_exit tracepoint and same thing for the clone child exit event. These 2 events are really important for the state of the so we implemented this workaround to catch this 2 missing events. We have instrumented 2 new tracepoints:

  • sched_proc_exec to catch the execve exit events. This tracepoint sends the same event PPME_SYSCALL_EXECVE_19_X to userspace, so the consumer won't notice any difference. This new tracepoint calls a new filler that is really similar to our actual proc_startupdate filler.
  • sched_proc_fork to catch the clone child exit events. Again we send the same event PPME_SYSCALL_CLONE_20_X and we have a new filler. The only change seen by userspace is that the clone child exit event will be received before the clone parent exit event, but this shouldn't be a problem since libsinsp already has the code to manage this situation.

Some important notes:

  • This solution is only for ARM64 and it is completely compiled out on other supported architectures (x86)
  • The only pieces of information we lose are the followings:
    • the clone child exit event has no flags information so param number 16 is always 0.
    • the clone child exit event gets euid and guid directly from the task struct without relating them to the user namespace (param numbers 17 and 18)
    • even if the system throws a clone3 the child exit event will be of type clone and not clone3 because we are not able to distinguish kernel side which syscall has been called. In the end, this doesn't change so much since the 2 events follow the same path both kernel side and userspace side.
  • fork and vfork are not defined in ARM64 so we don't lose information.
  • In case the execve or clone syscalls fail, we catch the exit events with our sys_exit tracepoint as usual, so we don't lose information at all in this case.

Compatibility:

  • BPF: we need kernel version >=4.17 since we need bpf raw_tracepoints programs to catch this new information. We lose 3 supported versions (x86 support BPF starting from 4.14) but we have no alternatives with this approach. If we want to support them we have to use kprobes.
  • KERNEL MODULE: we need kernel versions >=3.4 since sched/sched_process_exec is available only kernels >= 3.4

Kernel engineers are working to find solution as you can see here , in the meanwhile this could be a possible solution, let me know what to you think about that.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Next steps:

  • add further comments
  • test it on more machines (any help is well accepted).
  • remove the test commits since they are only useful to test the new patch (I removed the DCO from them, so we cannot merge the PR)
  • add logic to catch clone child flags.
  • remove the [WIP] tag.

--> One last note
We compiled out also the page_fault tracepoints both from BPF and kernel module since they are not supported on ARM64

Does this PR introduce a user-facing change?:

fix: enable `execve` exit events and `clone` child exit events on `ARM64` architecture

Andreagit97 and others added 11 commits June 21, 2022 13:34
@Andreagit97 @FedeDP
new(bpf): add support for raw_tracepoint in ARM64
3a92cb0 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Co-authored-by: Federico Di Pierro <nierro92@gmail.com>
@Andreagit97
new(bpf): add sched_process_exec raw tracepoint
be0774c 
We implemented this tracepoint to catch `execve` exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
cleanup(scap_bpf_engine): remove useless check on fillers implementation
a185cca 
Removing this check, we can compile out fillers that are not used in that specific architecture

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
new(bpf): add sched_process_fork raw tracepoint
90fc567 
This tracepoint catches `clone` child exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
new(kmod): add sched_process_exec tracepoint
00d1650 
This tracepoint catches `execve` exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
new(kmod): add sched_process_fork tracepoint
272c3d8 
This tracepoint catches `clone` child exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
update(bpf, kmod): ignore kernel threads
a10b23e 
We are not interested in catching information about kernel threads

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
cleanup(kmod): use kernel config CONFIG_ARM64 to compile out
46309ed 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
cleanup(bpf): use clang pre-processor macros to compile out
8f94ab9 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
cleanup(scap_bpf_engine): remove duplicate definitions of BPF macros
0c7fa56 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
update(kmod): compile out page_fault tracepoints in ARM64
865a9a4 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@poiana poiana requested review from gnosek and mstemm June 21, 2022 13:43
@Andreagit97
Copy link
Copy Markdown
Member Author

Andreagit97 commented Jun 21, 2022

I removed the DCO from the last two commits since we have to remove them before merging the PR

FedeDP
FedeDP reviewed Jun 21, 2022
View reviewed changes
FedeDP
FedeDP reviewed Jun 21, 2022
View reviewed changes
FedeDP
FedeDP reviewed Jun 21, 2022
View reviewed changes
@FedeDP
Copy link
Copy Markdown
Contributor

FedeDP commented Jun 22, 2022

What about adding a README section with supported architectures and their kernel versions requirements for probe and kmod?
Thank you!

@Andreagit97
Copy link
Copy Markdown
Member Author

Andreagit97 commented Jun 22, 2022

What about adding a README section with supported architectures and their kernel versions requirements for probe and kmod? Thank you!

I will open another PR, with also a Cmake check on supported architectures, if this is ok for you :)

@Andreagit97
Copy link
Copy Markdown
Member Author

Andreagit97 commented Jun 22, 2022

I will open another PR, with also a Cmake check on supported architectures, if this is ok for you :)

This is the related pull request -> #421

@poiana poiana added the lgtm label Jun 27, 2022
@poiana
Copy link
Copy Markdown
Contributor

poiana commented Jun 27, 2022

LGTM label has been added.

DetailsGit tree hash: a3b9f13fa4f2464e93f57d41d72e82d8f63d146d

@Andreagit97
refactor(bpf,kmod): rename child struct into child
f38eb4b 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
update(bpf,kmod): check pidns!=init_ns
9154d36 
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@poiana poiana removed the lgtm label Jun 27, 2022
@poiana poiana requested a review from FedeDP June 27, 2022 20:41
@FedeDP
Copy link
Copy Markdown
Contributor

FedeDP commented Jun 28, 2022

/test build-libs-bundled-deps

FedeDP
FedeDP approved these changes Jun 28, 2022
View reviewed changes
Copy link
Copy Markdown
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 poiana added the lgtm label Jun 28, 2022
@poiana
Copy link
Copy Markdown
Contributor

poiana commented Jun 28, 2022

LGTM label has been added.

DetailsGit tree hash: 813e46e1d98b6cb7a22dcdfbe0eddc61d0608360

Andreagit97
Andreagit97 commented Jun 28, 2022
View reviewed changes
driver/bpf/fillers.h
Comment on lines +6409 to +6412
if(pidns_level != 0)
{
flags |= PPM_CL_CHILD_IN_PIDNS;
}
Copy link
Copy Markdown
Member Author

@Andreagit97 Andreagit97 Jun 28, 2022

Choose a reason for hiding this comment

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

In the end, we want to detect every child that is spawned in a PID namespace different from the init one so we can keep only this logic in the if statement

@FedeDP FedeDP mentioned this pull request Jun 28, 2022
Merged
@Andreagit97
Copy link
Copy Markdown
Member Author

Andreagit97 commented Jun 28, 2022

Hi @gnosek, are these changes ok for you?

@deepskyblue86
Copy link
Copy Markdown
Member

deepskyblue86 commented Jun 29, 2022

CC @jcpittman144, PTAL

leogr
leogr approved these changes Jun 30, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM

@gnosek
Copy link
Copy Markdown
Contributor

gnosek commented Jun 30, 2022

/approve

@poiana
Copy link
Copy Markdown
Contributor

poiana commented Jun 30, 2022

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [Andreagit97,FedeDP,gnosek]

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 010bf3c into master Jun 30, 2022
@poiana poiana deleted the fix_arm_registers branch June 30, 2022 15:47
@FedeDP FedeDP mentioned this pull request Jul 1, 2022
Merged
@Andreagit97 Andreagit97 mentioned this pull request Jul 4, 2022
Merged
@Andreagit97 Andreagit97 mentioned this pull request Jul 23, 2022
Merged
@incertum incertum mentioned this pull request Sep 20, 2022
Closed
leogr pushed a commit to leogr/libs that referenced this pull request Jan 5, 2023
@jcpittman144 @leogr
Incorporate ARM support changes from upstream falcosecurity/libs repo (…
1077e10 
…falcosecurity#93)

- Falcosecurity/libs falcosecurity#416: Support execve exit and clone child exit events on ARM64
- Falcosecurity/libs falcosecurity#418: Enable 64BIT_ARGS_SINGLE_REGISER on ARM64
- Also, disable userspace workarounds ARM, which attempted to compensate
  for the missing execve/clone exit events
@deepskyblue86
Copy link
Copy Markdown
Member

deepskyblue86 commented Nov 17, 2025

Kernel engineers are working to find solution as you can see here , in the meanwhile this could be a possible solution, let me know what to you think about that.

Looks like this was eventually addressed in 6.13: torvalds/linux@f260c4426763#diff-ff1e8aca5d52c7e9770b246fc6f5af681a4267169df077020569051615bfad69R301

@ekoops
Copy link
Copy Markdown
Contributor

ekoops commented Nov 20, 2025

AFAIK, this was addressed in kernel 5.18 and backported till 5.15 @deepskyblue86

This was referenced Nov 25, 2025
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leogr leogr leogr approved these changes

@FedeDP FedeDP FedeDP approved these changes

@gnosek gnosek Awaiting requested review from gnosek

@mstemm mstemm Awaiting requested review from mstemm

Assignees

@leogr leogr

@FedeDP FedeDP

Labels

approved area/driver-bpf area/driver-kmod area/libscap-engine-bpf dco-signoff: yes kind/bug Something isn't working kind/feature New feature or request lgtm release-note size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Andreagit97 @FedeDP @poiana @deepskyblue86 @gnosek @ekoops @leogr