falcosecurity / libs

libsinsp, libscap, the kernel module driver, and the eBPF driver sources
https://falcosecurity.github.io/libs/
Apache License 2.0
212 stars 159 forks source link

Uninitialized efd field in alloc_handle() leads to undefined behavior #1448

Closed NitroCao closed 8 months ago

NitroCao commented 8 months ago

Describe the bug https://github.com/falcosecurity/libs/blob/56b6e591cf8bf6849c27adfccc1557835daa1529/userspace/libscap/engine/bpf/scap_bpf.c#L118-L135 https://github.com/falcosecurity/libs/blob/56b6e591cf8bf6849c27adfccc1557835daa1529/userspace/libscap/engine/bpf/attached_prog.c#L215-L224 In current code, alloc_handle() doesn't initialize engine->m_attached_progs[j].efd to -1, which leads to detach_bpf_prog() closing those file descriptors which is equal to 0 unexpectedly.

How to reproduce it The bug was first found in latest version of Falco. Falco will monitor the directories stored rules using inotify and reload rules automatically when modifications are made. To reproduce this bug, just run the latest version of Falco and modify the rules specified in configuration file. Falco would restart and reload rules at the first time. Then we'll get the error message Failed read with inotify handler, shutting down watcher.... and the monitor doesn't work anymore, Falco wouldn't reload rules if we make any modification. If we check entries in /proc/[PID]/fd, we couldn't find any inotify file descriptor.

Expected behaviour

Screenshots

Environment

REDHAT_BUGZILLA_PRODUCT="Red Hat Enterprise Linux 8" REDHAT_BUGZILLA_PRODUCT_VERSION=8.8 REDHAT_SUPPORT_PRODUCT="Red Hat Enterprise Linux" REDHAT_SUPPORT_PRODUCT_VERSION="8.8"


<!-- Eg., output of "cat /etc/os-release". -->
- Kernel: Linux magma.localdomain 4.18.0-477.10.1.el8_8.x86_64 #1 SMP Wed Apr 5 13:35:01 EDT 2023 x86_64 x86_64 x86_64 GNU/Linux
<!-- Eg., output of "uname -a". -->
- Installation method: from source
<!-- Eg., Kubernetes, RPM, DEB, from source? -->

**Additional context**
![CleanShot 2023-10-26 at 14 37 20@2x](https://github.com/falcosecurity/libs/assets/17915615/37d54253-5432-4f91-b4f0-19a367c78c7f)
This stack trace is at the first `close(0)` function after `inotify_init()` in `falco::app::restart_handler::start()` when reloading rules.
<!-- Add any other context about the problem here. -->
FedeDP commented 8 months ago

Hi!

In current code, alloc_handle() doesn't initialize engine->m_attached_progs[j].efd to -1, which leads to detach_bpf_prog() closing those file descriptors which is equal to 0 unexpectedly.

This is a great catch; thank you very much for spending the time to discover this issue. Would you be open to patch it through a PR? We are approaching a 0.36.2 patch release for Falco; this fix would be great!

FedeDP commented 8 months ago

/milestone 0.13.4