libs: [Tracking] Params inconsistencies in our drivers

PLEASE NOTE

This issue is mainly for tracking purposes, some points cannot be addressed until we solve the scap-file compatibility issue -> https://github.com/falcosecurity/libs/pull/1381#issuecomment-1746613905

Generic context

The aim of this issue is to track all the inconsistencies when we send event params from our drivers (modern_bpf, bpf, kernel module) to userspace. Some widespread issues that need a dedicated conversion

  • Today when we send file descriptors fd to userspace, we send them as int64_t while they are represented on int32_t. This leads us to waste a lot of space in our ring buffers… fd params are very common in our event, we waste 4 bytes every time we send a param of this type. Considering a small/medium-size system, we can imagine that it could send also 1 million of fd params per second, this would mean wasting almost 4 MB of space in our ring buffers per second!
  • Today when we send process identifiers pid to userspace, we send them as int64_t while they are represented on int32_t. This leads us to waste a lot of space in our ring buffers as explained in the previous fd case.
  • In some syscalls we take the syscalls flags as an int value and we push it to userspace as uint32_t without converting it with our internal PPM representation.
  • This is similar to the previous point, but even if we send flags/modes with the same type (so take uint32_t and send uint32_t) in some cases we don’t convert these values into the scap PPM format, so we cannot use this flags/modes userspace-side even if we catch them driver side.
  • In some events we send empty params because they are still not implemented.
  • Every syscall must have its PPM_CODE and its event pair.
  • Different drivers manage max boundaries in different ways we need to uniform them in some way 👇 https://github.com/falcosecurity/libs/pull/648#discussion_r996388593

Syscall-specific issues

LEGENDA

  • [NOT ADDRESSABLE] -> means that the issue is not addressable at the moment, at least until we don’t solve the scap file issue, see PLEASE NOTE ☝️
  • [MODERN_BPF] -> means that the issue is only related to the modern_bpf probe
  • [BPF] -> means that the issue is only related to the bpf probe
  • [KMOD] -> means that the issue is only related to the kernel module
  • ⚠️ -> possible problems, fix it!
  • ⬅️ -> only in the enter event
  • ➡️ -> only in the exit event

open_by_handle_at ➡️

  • [MODERN BPF] we can get at maximum 8 path_components we need to find a workaround to manage more components!
  • open_flags_to_scap() method should receive an int value and not a uint32_t.

dup3 ➡️

  • dup3_flags_to_scap() method should receive an int value and not a uint32_t.

open

  • open_flags_to_scap() method should receive an int value and not a uint32_t.

openat

  • open_flags_to_scap() method should receive an int value and not a uint32_t.

openat2

  • open_flags_to_scap() method should receive an int value and not a uint32_t.

eventfd ⬅️

eventfd2 ⬅️

  • param 2 (flags) is not implemented, we push 0 to userspace

inotify_init ⬅️

  • [NOT ADDRESSABLE] inotify_init has no syscall arguments but we send one param

signalfd ⬅️

  • [NOT ADDRESSABLE] param 2 (mask) is not implemented, we push 0 to userspace. We should remove it
  • [NOT ADDRESSABLE] param 3 (flags)is not implemented, we push 0 to userspace. We should remove it. Moreover, this syscall has not a flag argument, please see here for more details https://elixir.bootlin.com/linux/v6.5.5/source/fs/signalfd.c#L314

signalfd4 ⬅️

  • [NOT ADDRESSABLE] param 2 (mask) is not implemented, we push 0 to userspace. We should remove it.

timerfd_create ⬅️

  • param 1 (clockid) is not implemented, we push 0 to userspace. We should implement it.
  • param 2 (flags) is not implemented, we push 0 to userspace. We should implement it.

userfault_fd ➡️

  • param 2 (flags) miss an helper like userfaultfd_flags_to_scap to convert flags to scap notation.

ptrace ➡️

  • [NOT ADDRESSABLE] param 2 (addr) not sure we really need a PT_DYN param, we always send the same len.
  • [NOT ADDRESSABLE] param 3 (data) not sure about the utlity of sending the data_pointer to userspace.

mkdirat ➡️

  • param 4 (mode) we need to convert the mode to the scap format.

pipe2

  • we need a new event for pipe2 otherwise we cannot catch the flags. Right now we use the same event of pipe.

renameat2 ➡️

  • param 6 (flags) we need to convert the flags to the scap format with an helper like renameat2_flags_to_scap.

execve ➡️

execveat ➡️

fork ➡️

clone ➡️

clone3 ➡️

vfork ➡️

socket ⬅️

  • [NOT ADDRESSABLE] param 1 (domain) the socket_family_to_scap method should receive an int, not a u8, and we need to choose if the param should be on 8 bits or 32 bits. We need also to update the socket_family_to_scap with new socket families.

connect ➡️

  • [NOT ADDRESSABLE] param 2 (tuple) in case of UNIX sockets, not sure about the utility of sending kernel pointers to userspace

socketpair ⬅️

Same issues of socket syscall

  • [NOT ADDRESSABLE] param 1 (domain) the socket_family_to_scap method should receive an int, not a u8, and we need to choose if the param should be on 8 bits or 32 bits. We need also to update the socket_family_to_scap with new socket families.

socketpair ➡️

  • [NOT ADDRESSABLE] param 4 (source) not sure about the utility of sending kernel pointers to userspace
  • [NOT ADDRESSABLE] param 5 (peer) not sure about the utility of sending kernel pointers to userspace

accept ➡️

accept4 ⬅️

  • [NOT ADDRESSABLE] param 1 (flags) still to implement, today we send always 0. This bug is used in the socketcall wokraround

listen ⬅️

bpf ⬅️

  • [NOT ADDRESSABLE] param 1 (cmd) is an int not a int64_t

flock ⬅️

  • param 2 (operation) we need to read it as an int and then convert it to uint32_t, while today we read it as an unsigned long

quotactl ⬅️

  • param 1 (cmd) we need to read it as an int and then convert it to uint32_t, while today we read it as an unsigned long
  • param 3 (id) is an int not a int32_t

quotactl ➡️

  • param 13 (dqi_flags) add conversion to scap format

unshare ⬅️

  • param 1 (flags) we need to read it as an int and then convert it to uint32_t, while today we read it as an unsigned long

mount ⬅️

  • param 1 (flags) if we want to use this info in userspace we need to convert it into scap format.

umount2 ⬅️

  • param 1 (flags) if we want to use this info in userspace we need to convert it into scap format. This field should be an int not a int32_t, https://github.com/falcosecurity/libs/pull/1255
  • we need to define a new event pair (PPME_SYSCALL_UMOUNT2_E, PPME_SYSCALL_UMOUNT2_X)

linkat ➡️

  • param 6 (flags) we need to read it as an int and then convert it to uint32_t, while today we read it as an unsigned long

unlinkat ➡️

  • param 4 (flags) we need to read it as an int and then convert it to uint32_t, while today we read it as an unsigned long

setns ⬅️

  • param 2 (nstype) we need to read it as an int and then convert it to uint32_t, while today we read it as an unsigned long

setrlimit ⬅️

  • param 1 (resource) we need to read it as an int and then convert it to uint8_t, while today we read it as an unsigned long

prlimit64 ⬅️

  • param 2 (resource) we need to read it as an int and then convert it to uint8_t, while today we read it as an unsigned long

sendto ⬅️

  • [NOT ADDRESSABLE] param 3 (tuple) should be catched in the exit event when we know the outcome of the syscall otherwise there is the risk to catch something wrong.

sendmsg ⬅️

  • [NOT ADDRESSABLE] param 3 (tuple) should be catched in the exit event when we know the outcome of the syscall otherwise there is the risk to catch something wrong.

ppoll⬅️

  • [NOT ADDRESSABLE] param 3 (sigmask) we send only the first 32 bits

ppoll⬅️

  • [NOT ADDRESSABLE] param 3 (sigmask) we send only the first 32 bits

ppoll⬅️

  • [NOT ADDRESSABLE] param 3 (sigmask) we send only the first 32 bits

recvmmsg:

[NOT ADDRESSABLE] Empty instrumentation

sendmmsg:

[NOT ADDRESSABLE] Empty instrumentation

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Reactions: 8
  • Comments: 21 (21 by maintainers)

Commits related to this issue

Most upvoted comments

First 2 points will be addressed by #526

+2
FedeDP on Aug 2, 2022

@incertum @Andreagit97 Can you guys also mark setns, flock, and unshare as complete?

+1
ecbadeaux on Dec 16, 2023

I’ve added the [NOT ADDRESSABLE] marker to all issues that we cannot address now due to the scap-file management https://github.com/falcosecurity/libs/pull/1381#issuecomment-1746613905

+1
Andreagit97 on Oct 4, 2023

“if we today are able to push to userspace 1mln fd-only events, we could instead push up to 2mln fd-only events” This hits even harder 😃

Optimism is awesome but let me cool it down a little bit 😛 Every event has this header, which is somewhat larger than the zero bytes it would need for that claim to be true 😉

struct ppm_evt_hdr {
#ifdef PPM_ENABLE_SENTINEL
    uint32_t sentinel_begin;
#endif
    uint64_t ts; /* timestamp, in nanoseconds from epoch */
    uint64_t tid; /* the tid of the thread that generated this event */
    uint32_t len; /* the event len, including the header */
    uint16_t type; /* the event type */
    uint32_t nparams; /* the number of parameters of the event */
};

BTW, we could probably easily change nparams to 16 bits. If we do expect >64k parameters, we can hack something for the (hopefully rare) events that exceed this number.

We could also trim the tid to 32 bits, but then we use this struct all over userspace too (#sadpanda), and other environments may want large tids (e.g. gvisor), so we would have to decouple these two structs and copy data field by field between them.

Don’t let me distract you from tracking down the inconsistencies though, that’s an awesome job!

These two changes would cut down 6 bytes from every event, equivalent to one and a half fds with no schema changes, just a major api version bump.

+1
gnosek on Dec 2, 2022

I have slightly changed the issue format with Generic event issues and Specific event issues in this way it should be more maintainable, thank you to @hbrueckner @Molter73 @FedeDP for all the help in finding new issues

+1
Andreagit97 on Aug 2, 2022
Read more comments on GitHub
← libs: [REGRESSION] Modern bpf probe in least privileged mode
libs: Unexpected number of processors (on musl build) →