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

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

Merged
merged 18 commits into from
Jun 30, 2022

Conversation

Andreagit97
Copy link
Member

@Andreagit97 Andreagit97 commented Jun 21, 2022

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
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Co-authored-by: Federico Di Pierro <nierro92@gmail.com>
We implemented this tracepoint to catch `execve` exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
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>
This tracepoint catches `clone` child exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
This tracepoint catches `execve` exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
This tracepoint catches `clone` child exit events on `arm64`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
We are not interested in catching information about kernel threads

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
Copy link
Member Author

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

driver/main.c Show resolved Hide resolved
driver/main.c Outdated Show resolved Hide resolved
@FedeDP
Copy link
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
Member Author

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
Member Author

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

@Andreagit97 Andreagit97 changed the title [WIP] Support execve exit events and clone child exit events on ARM64 Support execve exit events and clone child exit events on ARM64 Jun 24, 2022
FedeDP
FedeDP previously approved these changes Jun 27, 2022
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
Contributor

poiana commented Jun 27, 2022

LGTM label has been added.

Git tree hash: a3b9f13fa4f2464e93f57d41d72e82d8f63d146d

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@FedeDP
Copy link
Contributor

FedeDP commented Jun 28, 2022

/test build-libs-bundled-deps

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 poiana added the lgtm label Jun 28, 2022
@poiana
Copy link
Contributor

poiana commented Jun 28, 2022

LGTM label has been added.

Git tree hash: 813e46e1d98b6cb7a22dcdfbe0eddc61d0608360

Comment on lines +6409 to +6412
if(pidns_level != 0)
{
flags |= PPM_CL_CHILD_IN_PIDNS;
}
Copy link
Member Author

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

@Andreagit97
Copy link
Member Author

Hi @gnosek, are these changes ok for you?

@deepskyblue86
Copy link
Contributor

CC @jcpittman144, PTAL

Copy link
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
Contributor

gnosek commented Jun 30, 2022

/approve

@poiana
Copy link
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

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
leogr pushed a commit to leogr/libs that referenced this pull request Jan 5, 2023
…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
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.

6 participants