From b7ad651b1c7ea9e392f1b9b4eb6d3072152644bd Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Wed, 4 Oct 2023 04:24:34 +0000 Subject: [PATCH 1/6] fix(userspace/falco): timer_delete() workaround due to bug in older GLIBC Workaround for older GLIBC versions (< 2.35), where calling timer_delete() with an invalid timer ID not returned by timer_create() causes a segfault because of a bug in GLIBC (https://sourceware.org/bugzilla/show_bug.cgi?id=28257). Signed-off-by: Melissa Kilby --- userspace/falco/stats_writer.cpp | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/userspace/falco/stats_writer.cpp b/userspace/falco/stats_writer.cpp index dc83ab9cb13..c1fb664879f 100644 --- a/userspace/falco/stats_writer.cpp +++ b/userspace/falco/stats_writer.cpp @@ -34,6 +34,10 @@ limitations under the License. // check that this value changed since their last observation. static std::atomic s_timer((stats_writer::ticker_t) 0); static timer_t s_timerid; +// note: Workaround for older GLIBC versions (< 2.35), where calling timer_delete() +// with an invalid timer ID not returned by timer_create() causes a segfault because of +// a bug in GLIBC (https://sourceware.org/bugzilla/show_bug.cgi?id=28257). +bool s_timerid_exists = false; static void timer_handler(int signum) { @@ -60,18 +64,31 @@ bool stats_writer::init_ticker(uint32_t interval_msec, std::string &err) sev.sigev_value.sival_ptr = &s_timerid; #ifndef __EMSCRIPTEN__ // delete any previously set timer - timer_delete(s_timerid); - if (timer_create(CLOCK_MONOTONIC, &sev, &s_timerid) == -1) { + if (s_timerid_exists) + { + if (timer_delete(s_timerid) == -1) + { + err = std::string("Could not delete previous timer: ") + strerror(errno); + return false; + } + s_timerid_exists = false; + } + + if (timer_create(CLOCK_MONOTONIC, &sev, &s_timerid) == -1) + { err = std::string("Could not create periodic timer: ") + strerror(errno); return false; } + s_timerid_exists = true; + #endif timer.it_value.tv_sec = interval_msec / 1000; timer.it_value.tv_nsec = (interval_msec % 1000) * 1000 * 1000; timer.it_interval = timer.it_value; #ifndef __EMSCRIPTEN__ - if (timer_settime(s_timerid, 0, &timer, NULL) == -1) { + if (timer_settime(s_timerid, 0, &timer, NULL) == -1) + { err = std::string("Could not set up periodic timer: ") + strerror(errno); return false; } @@ -134,7 +151,10 @@ stats_writer::~stats_writer() } // delete timerID and reset timer #ifndef __EMSCRIPTEN__ - timer_delete(s_timerid); + if (s_timerid_exists) + { + timer_delete(s_timerid); + } #endif } } From a042e084814b61cd52b836ca7f327b9cfe1fd867 Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Wed, 4 Oct 2023 14:21:27 +0000 Subject: [PATCH 2/6] cleanup(userspace/falco): add more comments around timer_delete workaround Co-authored-by: Federico Di Pierro Signed-off-by: Melissa Kilby --- userspace/falco/stats_writer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/userspace/falco/stats_writer.cpp b/userspace/falco/stats_writer.cpp index c1fb664879f..a71a597993e 100644 --- a/userspace/falco/stats_writer.cpp +++ b/userspace/falco/stats_writer.cpp @@ -37,6 +37,8 @@ static timer_t s_timerid; // note: Workaround for older GLIBC versions (< 2.35), where calling timer_delete() // with an invalid timer ID not returned by timer_create() causes a segfault because of // a bug in GLIBC (https://sourceware.org/bugzilla/show_bug.cgi?id=28257). +// Just performing a nullptr check is not enough as even after creating the timer, s_timerid +// remains a nullptr somehow. bool s_timerid_exists = false; static void timer_handler(int signum) From a219c4a05bc04a3cb81e6c4ce9df1f68afce6a8f Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Wed, 4 Oct 2023 14:28:28 +0000 Subject: [PATCH 3/6] chore: apply codespell fixes Signed-off-by: Melissa Kilby --- CHANGELOG.md | 4 ++-- proposals/20221129-artifacts-distribution.md | 2 +- userspace/falco/atomic_signal_handler.h | 4 ++-- userspace/falco/falco_outputs.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb57578f361..cc2431eb539 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ Released on 2023-09-25 * new(falco-driver-loader): --source-only now prints the values as env vars [[#2353](https://github.com/falcosecurity/falco/pull/2353)] - [@steakunderscore](https://github.com/steakunderscore) -* new(docker): allow passing options to falco-driver-loader from the driver loader cointainer [[#2781](https://github.com/falcosecurity/falco/pull/2781)] - [@LucaGuerra](https://github.com/LucaGuerra) +* new(docker): allow passing options to falco-driver-loader from the driver loader container [[#2781](https://github.com/falcosecurity/falco/pull/2781)] - [@LucaGuerra](https://github.com/LucaGuerra) * new(docker): add experimental falco-distroless image based on Wolfi [[#2768](https://github.com/falcosecurity/falco/pull/2768)] - [@LucaGuerra](https://github.com/LucaGuerra) * new: the legacy falco image is available as driver-loader-legacy [[#2718](https://github.com/falcosecurity/falco/pull/2718)] - [@LucaGuerra](https://github.com/LucaGuerra) * new: added option to enable/disable echoing of server answer to stdout (disabled by default) when using HTTP output [[#2602](https://github.com/falcosecurity/falco/pull/2602)] - [@FedeDP](https://github.com/FedeDP) @@ -1086,7 +1086,7 @@ Released on 2021-01-18 ### Minor Changes * build: bump b64 to v2.0.0.1 [[#1441](https://github.com/falcosecurity/falco/pull/1441)] - [@fntlnz](https://github.com/fntlnz) -* rules(macro container_started): re-use `spawned_process` macro inside `container_started` macro [[#1449](https://github.com/falcosecurity/falco/pull/1449)] - [@leodido](https://github.com/leodido) +* rules(macro container_started): reuse `spawned_process` macro inside `container_started` macro [[#1449](https://github.com/falcosecurity/falco/pull/1449)] - [@leodido](https://github.com/leodido) * docs: reach out documentation [[#1472](https://github.com/falcosecurity/falco/pull/1472)] - [@fntlnz](https://github.com/fntlnz) * docs: Broken outputs.proto link [[#1493](https://github.com/falcosecurity/falco/pull/1493)] - [@deepskyblue86](https://github.com/deepskyblue86) * docs(README.md): correct broken links [[#1506](https://github.com/falcosecurity/falco/pull/1506)] - [@leogr](https://github.com/leogr) diff --git a/proposals/20221129-artifacts-distribution.md b/proposals/20221129-artifacts-distribution.md index f3cfd907870..4c6a96eb9ca 100644 --- a/proposals/20221129-artifacts-distribution.md +++ b/proposals/20221129-artifacts-distribution.md @@ -69,7 +69,7 @@ The allowed publishing channels are: Both channels are equivalent and may publish the same artifacts. However, for historical reasons and to avoid confusion, the **`docker.io` registry should only be used for container images** and not for other kinds of artifacts (e.g., plugins, rules, etc.). -Mirrors are allowed and encouraged if they facilitate artifacts consumption by our users. This proposal reccomends to enable mirrors on the major public OCI registry, such as [Amazon ECR](https://gallery.ecr.aws/) (which is already implentend in our infra at the time of writing). +Mirrors are allowed and encouraged if they facilitate artifacts consumption by our users. This proposal recommends to enable mirrors on the major public OCI registry, such as [Amazon ECR](https://gallery.ecr.aws/) (which is already implentend in our infra at the time of writing). Official **channels and mirrors must be listed at [falco.org](https://falco.org/)**. diff --git a/userspace/falco/atomic_signal_handler.h b/userspace/falco/atomic_signal_handler.h index 001f15310a0..23147306049 100644 --- a/userspace/falco/atomic_signal_handler.h +++ b/userspace/falco/atomic_signal_handler.h @@ -87,9 +87,9 @@ namespace falco /** * @brief If a signal is triggered, performs an handler action. * The action function will be invoked exactly once among all the - * simultaneus calls. The action will not be performed if the + * simultaneous calls. The action will not be performed if the * signal is not triggered, or if the triggered has already been - * handled. When an action is being performed, all the simultaneus + * handled. When an action is being performed, all the simultaneous * callers will wait and be blocked up until its execution is finished. * If the handler action throws an exception, it will be considered * performed. After the first handler has been performed, every diff --git a/userspace/falco/falco_outputs.h b/userspace/falco/falco_outputs.h index 6a9c8b3cc8e..d5a243a2ce7 100644 --- a/userspace/falco/falco_outputs.h +++ b/userspace/falco/falco_outputs.h @@ -36,7 +36,7 @@ limitations under the License. All methods in this class are thread-safe. The output framework supports a multi-producer model where messages are stored in a queue and consumed - by each configured output asynchrounously. + by each configured output asynchronously. */ class falco_outputs { From 5a8f0b713da8dd5448553ed7febcb8334e00be2e Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Thu, 5 Oct 2023 14:51:36 +0000 Subject: [PATCH 4/6] cleanup(userspace/falco): reset s_timerid_exists at stats_writer teardown Co-authored-by: Andrea Terzolo Signed-off-by: Melissa Kilby --- userspace/falco/stats_writer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/userspace/falco/stats_writer.cpp b/userspace/falco/stats_writer.cpp index a71a597993e..0de4ed3856a 100644 --- a/userspace/falco/stats_writer.cpp +++ b/userspace/falco/stats_writer.cpp @@ -156,6 +156,7 @@ stats_writer::~stats_writer() if (s_timerid_exists) { timer_delete(s_timerid); + s_timerid_exists = false; } #endif } From 7e16c2bd8b5313ca77dbd2d724b27654782e8262 Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Tue, 10 Oct 2023 16:39:42 +0000 Subject: [PATCH 5/6] feat(userspace): remove experimental outputs queue recovery strategies Signed-off-by: Melissa Kilby --- falco.yaml | 19 +++++--------- userspace/engine/falco_common.cpp | 19 -------------- userspace/engine/falco_common.h | 7 ------ userspace/falco/app/actions/init_outputs.cpp | 1 - userspace/falco/configuration.cpp | 6 ----- userspace/falco/configuration.h | 1 - userspace/falco/falco_outputs.cpp | 26 +++----------------- userspace/falco/falco_outputs.h | 6 ++--- userspace/falco/stats_writer.cpp | 3 +-- 9 files changed, 12 insertions(+), 76 deletions(-) diff --git a/falco.yaml b/falco.yaml index 09bc110d0e3..4c3c994e3e9 100644 --- a/falco.yaml +++ b/falco.yaml @@ -331,24 +331,17 @@ rule_matching: first # If it does, it is most likely happening due to the entire event flow being too slow, # indicating that the server is under heavy load. # -# Lowering the number of items can prevent memory from steadily increasing until the OOM -# killer stops the Falco process. We provide recovery actions to self-limit or self-kill -# in order to handle this situation earlier, similar to how we expose the kernel buffer size -# as a parameter. However, it will not address the root cause of the event pipe not keeping up. -# # `capacity`: the maximum number of items allowed in the queue is determined by this value. # Setting the value to 0 (which is the default) is equivalent to keeping the queue unbounded. -# In other words, when this configuration is set to 0, the number of allowed items is effectively -# set to the largest possible long value, disabling this setting. +# In other words, when this configuration is set to 0, the number of allowed items is +# effectively set to the largest possible long value, disabling this setting. # -# `recovery`: strategy to follow when the queue becomes filled up. It applies only when the -# queue is bounded and there is still available system memory. In the case of an unbounded -# queue, if the available memory on the system is consumed, the Falco process would be -# OOM killed. The value `exit` is the default, `continue` does nothing special and `empty` -# empties the queue and then continues. +# In the case of an unbounded queue, if the available memory on the system is consumed, +# the Falco process would be OOM killed. When using this option and setting the capacity, +# the current event would be dropped, and the event loop would continue. This behavior mirrors +# kernel-side event drops when the buffer between kernel space and user space is full. outputs_queue: capacity: 0 - recovery: exit ########################## diff --git a/userspace/engine/falco_common.cpp b/userspace/engine/falco_common.cpp index f1c1fc139c2..8ed555d9e78 100644 --- a/userspace/engine/falco_common.cpp +++ b/userspace/engine/falco_common.cpp @@ -33,12 +33,6 @@ static std::vector rule_matching_names = { "all" }; -static std::vector outputs_queue_recovery_names = { - "continue", - "exit", - "empty", -}; - bool falco_common::parse_priority(std::string v, priority_type& out) { for (size_t i = 0; i < priority_names.size(); i++) @@ -66,19 +60,6 @@ falco_common::priority_type falco_common::parse_priority(std::string v) return out; } -bool falco_common::parse_queue_recovery(std::string v, outputs_queue_recovery_type& out) -{ - for (size_t i = 0; i < outputs_queue_recovery_names.size(); i++) - { - if (!strcasecmp(v.c_str(), outputs_queue_recovery_names[i].c_str())) - { - out = (outputs_queue_recovery_type) i; - return true; - } - } - return false; -} - bool falco_common::format_priority(priority_type v, std::string& out, bool shortfmt) { if ((size_t) v < priority_names.size()) diff --git a/userspace/engine/falco_common.h b/userspace/engine/falco_common.h index aa4f39e93e9..dd4f07b4566 100644 --- a/userspace/engine/falco_common.h +++ b/userspace/engine/falco_common.h @@ -60,12 +60,6 @@ struct falco_exception : std::exception namespace falco_common { - enum outputs_queue_recovery_type { - RECOVERY_CONTINUE = 0, /* outputs_queue_capacity recovery strategy of continuing on. */ - RECOVERY_EXIT = 1, /* outputs_queue_capacity recovery strategy of exiting, self OOM kill. */ - RECOVERY_EMPTY = 2, /* outputs_queue_capacity recovery strategy of emptying queue then continuing. */ - }; - const std::string syscall_source = sinsp_syscall_event_source_name; // Same as numbers/indices into the above vector @@ -83,7 +77,6 @@ namespace falco_common bool parse_priority(std::string v, priority_type& out); priority_type parse_priority(std::string v); - bool parse_queue_recovery(std::string v, outputs_queue_recovery_type& out); bool format_priority(priority_type v, std::string& out, bool shortfmt=false); std::string format_priority(priority_type v, bool shortfmt=false); diff --git a/userspace/falco/app/actions/init_outputs.cpp b/userspace/falco/app/actions/init_outputs.cpp index 2e8afbedd08..c690ec59b14 100644 --- a/userspace/falco/app/actions/init_outputs.cpp +++ b/userspace/falco/app/actions/init_outputs.cpp @@ -65,7 +65,6 @@ falco::app::run_result falco::app::actions::init_outputs(falco::app::state& s) s.config->m_output_timeout, s.config->m_buffered_outputs, s.config->m_outputs_queue_capacity, - s.config->m_outputs_queue_recovery, s.config->m_time_format_iso_8601, hostname)); diff --git a/userspace/falco/configuration.cpp b/userspace/falco/configuration.cpp index a884734a97b..513b3ef3d82 100644 --- a/userspace/falco/configuration.cpp +++ b/userspace/falco/configuration.cpp @@ -42,7 +42,6 @@ falco_configuration::falco_configuration(): m_watch_config_files(true), m_buffered_outputs(false), m_outputs_queue_capacity(DEFAULT_OUTPUTS_QUEUE_CAPACITY_UNBOUNDED_MAX_LONG_VALUE), - m_outputs_queue_recovery(falco_common::RECOVERY_EXIT), m_time_format_iso_8601(false), m_output_timeout(2000), m_grpc_enabled(false), @@ -290,11 +289,6 @@ void falco_configuration::load_yaml(const std::string& config_name, const yaml_h { m_outputs_queue_capacity = DEFAULT_OUTPUTS_QUEUE_CAPACITY_UNBOUNDED_MAX_LONG_VALUE; } - std::string recovery = config.get_scalar("outputs_queue.recovery", "exit"); - if (!falco_common::parse_queue_recovery(recovery, m_outputs_queue_recovery)) - { - throw std::logic_error("Unknown recovery \"" + recovery + "\"--must be one of exit, continue, empty"); - } m_time_format_iso_8601 = config.get_scalar("time_format_iso_8601", false); diff --git a/userspace/falco/configuration.h b/userspace/falco/configuration.h index 2a2ed20cfd8..ca2c4d2e93d 100644 --- a/userspace/falco/configuration.h +++ b/userspace/falco/configuration.h @@ -74,7 +74,6 @@ class falco_configuration bool m_watch_config_files; bool m_buffered_outputs; size_t m_outputs_queue_capacity; - falco_common::outputs_queue_recovery_type m_outputs_queue_recovery; bool m_time_format_iso_8601; uint32_t m_output_timeout; diff --git a/userspace/falco/falco_outputs.cpp b/userspace/falco/falco_outputs.cpp index 44b3ba31610..c14efcc6d1c 100644 --- a/userspace/falco/falco_outputs.cpp +++ b/userspace/falco/falco_outputs.cpp @@ -48,7 +48,6 @@ falco_outputs::falco_outputs( uint32_t timeout, bool buffered, size_t outputs_queue_capacity, - falco_common::outputs_queue_recovery_type outputs_queue_recovery, bool time_format_iso_8601, const std::string& hostname) { @@ -67,7 +66,6 @@ falco_outputs::falco_outputs( add_output(output); } m_outputs_queue_num_drops = {0}; - m_outputs_queue_recovery = outputs_queue_recovery; #ifndef __EMSCRIPTEN__ m_queue.set_capacity(outputs_queue_capacity); m_worker_thread = std::thread(&falco_outputs::worker, this); @@ -287,29 +285,11 @@ inline void falco_outputs::push(const ctrl_msg& cmsg) #ifndef __EMSCRIPTEN__ if (!m_queue.try_push(cmsg)) { - switch (m_outputs_queue_recovery) + if(m_outputs_queue_num_drops.load() == 0) { - case falco_common::RECOVERY_EXIT: - throw falco_exception("Fatal error: Output queue out of memory. Exiting ..."); - case falco_common::RECOVERY_EMPTY: - /* Print a log just the first time */ - if(m_outputs_queue_num_drops.load() == 0) - { - falco_logger::log(LOG_ERR, "Output queue out of memory. Drop event plus events in queue due to emptying the queue; continue on ..."); - } - m_outputs_queue_num_drops += m_queue.size() + 1; - m_queue.clear(); - break; - case falco_common::RECOVERY_CONTINUE: - if(m_outputs_queue_num_drops.load() == 0) - { - falco_logger::log(LOG_ERR, "Output queue out of memory. Drop event and continue on ..."); - } - m_outputs_queue_num_drops++; - break; - default: - throw falco_exception("Fatal error: strategy unknown. Exiting ..."); + falco_logger::log(LOG_ERR, "Outputs queue out of memory. Drop event and continue on ..."); } + m_outputs_queue_num_drops++; } #else for (auto o : m_outputs) diff --git a/userspace/falco/falco_outputs.h b/userspace/falco/falco_outputs.h index d5a243a2ce7..5e0577a8b36 100644 --- a/userspace/falco/falco_outputs.h +++ b/userspace/falco/falco_outputs.h @@ -50,7 +50,6 @@ class falco_outputs uint32_t timeout, bool buffered, size_t outputs_queue_capacity, - falco_common::outputs_queue_recovery_type outputs_queue_recovery, bool time_format_iso_8601, const std::string& hostname); @@ -87,8 +86,8 @@ class falco_outputs void reopen_outputs(); /*! - \brief Return the number of currently dropped events as a result of failed push attempts - into the outputs queue when using `continue` or `empty` recovery strategies. + \brief Return the number of events currently dropped due to failed push + attempts into the outputs queue */ uint64_t get_outputs_queue_num_drops(); @@ -121,7 +120,6 @@ class falco_outputs falco_outputs_cbq m_queue; #endif - falco_common::outputs_queue_recovery_type m_outputs_queue_recovery; std::atomic m_outputs_queue_num_drops; std::thread m_worker_thread; inline void push(const ctrl_msg& cmsg); diff --git a/userspace/falco/stats_writer.cpp b/userspace/falco/stats_writer.cpp index 0de4ed3856a..73e25950d84 100644 --- a/userspace/falco/stats_writer.cpp +++ b/userspace/falco/stats_writer.cpp @@ -132,8 +132,7 @@ stats_writer::stats_writer( if (m_initialized) { #ifndef __EMSCRIPTEN__ - // capacity and controls should not be relevant for stats outputs, adopt capacity - // for completeness, but do not implement config recovery strategies. + // Adopt capacity for completeness, even if it's likely not relevant m_queue.set_capacity(config->m_outputs_queue_capacity); m_worker = std::thread(&stats_writer::worker, this); #endif From 42bc933ccc33936f835e9ef5ba70044564eea0e5 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Fri, 13 Oct 2023 17:52:25 +0200 Subject: [PATCH 6/6] chore: bump libs to 0.13.2-rc1 tag Signed-off-by: Andrea Terzolo --- cmake/modules/falcosecurity-libs.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/modules/falcosecurity-libs.cmake b/cmake/modules/falcosecurity-libs.cmake index fbcf6e7a072..6f2e1182ccc 100644 --- a/cmake/modules/falcosecurity-libs.cmake +++ b/cmake/modules/falcosecurity-libs.cmake @@ -35,8 +35,8 @@ else() # In case you want to test against another falcosecurity/libs version (or branch, or commit) just pass the variable - # ie., `cmake -DFALCOSECURITY_LIBS_VERSION=dev ..` if(NOT FALCOSECURITY_LIBS_VERSION) - set(FALCOSECURITY_LIBS_VERSION "0.13.1") - set(FALCOSECURITY_LIBS_CHECKSUM "SHA256=2be42a27be3ffe6bd7e53eaa5d8358cab05a0dca821819c6e9059e51b9786219") + set(FALCOSECURITY_LIBS_VERSION "0.13.2-rc1") + set(FALCOSECURITY_LIBS_CHECKSUM "SHA256=2f5155bb81ee8d65ec76c20d55ebbfa1a196893fdee4450d63bcf873dfcbb90d") endif() # cd /path/to/build && cmake /path/to/source