From 641d1e6129b7f6ba19cf7a7a5a1edae92ce470b7 Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Tue, 31 Jan 2023 14:05:55 +0100 Subject: [PATCH 1/3] fix(ebpf): wait init before sending events On eBPF program startup we had errors -2 when writing to the perf event array. This happened because the userspace side of the map wasn't opened yet. Added a new map which contains the initialization status. --- crates/bpf-common/include/output.bpf.h | 19 +++++++++++++++++++ crates/bpf-common/src/program.rs | 16 +++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/crates/bpf-common/include/output.bpf.h b/crates/bpf-common/include/output.bpf.h index 995d5bf1..ecdaa5c1 100644 --- a/crates/bpf-common/include/output.bpf.h +++ b/crates/bpf-common/include/output.bpf.h @@ -47,6 +47,18 @@ struct { __uint(max_entries, 1); } temp_map SEC(".maps"); +// The status map contains configuration status for the eBPF program: +// - The STATUS_INITIALIZED entry indicates the perf event array initialization +// status. On startup it's set to 0, when the userspace program opens the perf +// event array it's set to 1. The eBPF program emits events only when it's 1. +#define STATUS_INITIALIZED 0 +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __type(key, u32); + __type(value, u32); + __uint(max_entries, 1); +} status_map SEC(".maps"); + // Get the temporary buffer inside temp_map as a void pointer, this can be cast // to the required event type and filled before submitting it for output. The // pointed at memory contiains BUFFER_MAX *2 bytes. @@ -63,6 +75,13 @@ static __always_inline void *output_temp() { static __always_inline void output_event(void *ctx, void *output_map, void *event, int struct_len, int buffer_len) { + u32 key = STATUS_INITIALIZED; + u32 *initialization_status = bpf_map_lookup_elem(&status_map, &key); + if (!initialization_status || !*initialization_status) { + // The userspace is not ready yet for events on the perf event array + return; + } + // The output size is the full struct length, minus the unused buffer len unsigned int len = struct_len - (BUFFER_MAX - buffer_len); if (len > 0 && len <= struct_len) { diff --git a/crates/bpf-common/src/program.rs b/crates/bpf-common/src/program.rs index 5263c4ec..f8555033 100644 --- a/crates/bpf-common/src/program.rs +++ b/crates/bpf-common/src/program.rs @@ -8,7 +8,7 @@ use std::{convert::TryFrom, fmt::Display, mem::size_of, sync::Arc, time::Duratio use aya::{ maps::{ perf::{AsyncPerfEventArray, PerfBufferError}, - HashMap, MapData, + Array, HashMap, MapData, }, programs::{KProbe, Lsm, RawTracePoint, TracePoint}, util::online_cpus, @@ -338,6 +338,11 @@ impl Program { .take_map(map_name) .ok_or_else(|| ProgramError::MapNotFound(map_name.to_string()))?; let mut perf_array: AsyncPerfEventArray<_> = AsyncPerfEventArray::try_from(map_resource)?; + let mut status_map = Array::try_from( + self.bpf + .take_map("status_map") + .ok_or_else(|| ProgramError::MapNotFound("status_map".to_string()))?, + )?; let buffers = online_cpus() .unwrap() @@ -394,6 +399,15 @@ impl Program { } }); } + + // Signal eBPF program we're ready by setting STATUS_INITIALIZED + if let Err(err) = status_map.set(0, 1_u32, 0) { + log::warn!( + "Error setting STATUS_INITIALIZED for {}: {:?}", + self.name, + err + ); + }; Ok(()) } } From 43a118f9b0e41925b00d7db36db3d2b2966a322f Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Tue, 31 Jan 2023 14:07:23 +0100 Subject: [PATCH 2/3] fix(pulsar): stop modules before exiting During shutdown, we got several errors in perf event output. -28 ENOSPC: No space left on device This happened because we were shutting down the userspace before the eBPF program was unloaded. --- crates/modules/file-system-monitor/src/lib.rs | 2 -- src/pulsard/mod.rs | 10 +++++++++- src/pulsard/module_manager.rs | 10 ++++++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/modules/file-system-monitor/src/lib.rs b/crates/modules/file-system-monitor/src/lib.rs index 15f21d61..cc7868fd 100644 --- a/crates/modules/file-system-monitor/src/lib.rs +++ b/crates/modules/file-system-monitor/src/lib.rs @@ -18,7 +18,6 @@ pub async fn program( // If LSM eBPF programs is not supported, we'll attach to the same kernel // functions, but using kprobes. if lsm_supported().await { - log::info!("Loading LSM programs"); builder = builder .lsm("path_mknod") .lsm("path_unlink") @@ -29,7 +28,6 @@ pub async fn program( .lsm("path_link") .lsm("path_symlink"); } else { - log::info!("LSM programs not supported. Falling back to kprobes"); builder = builder .kprobe("security_path_mknod") .kprobe("security_path_unlink") diff --git a/src/pulsard/mod.rs b/src/pulsard/mod.rs index 90e62f2c..9bf37594 100644 --- a/src/pulsard/mod.rs +++ b/src/pulsard/mod.rs @@ -64,7 +64,15 @@ pub async fn pulsar_daemon_run( server_handle.stop().await; log::info!("Terminating Pulsar Daemon..."); - drop(pulsar_daemon); + for module in pulsar_daemon.modules().await { + if let Err(err) = pulsar_daemon.stop(module.name.clone()).await { + log::warn!( + "Module {} didn't respond to shutdown signal. Forcing shutdown.\n{:?}", + module.name, + err + ) + } + } Ok(()) } diff --git a/src/pulsard/module_manager.rs b/src/pulsard/module_manager.rs index 11dee4c8..19e34b80 100644 --- a/src/pulsard/module_manager.rs +++ b/src/pulsard/module_manager.rs @@ -134,8 +134,14 @@ impl ModuleManager { let (tx_shutdown, task) = self.running_task.take().unwrap(); tx_shutdown.send_signal(); let result = task.await; - log::info!("Module {} exited: {:?}", self.task_launcher.name(), result); - // TODO: use this result and report errors + match result { + Ok(()) => log::info!("Module {} exited", self.task_launcher.name()), + Err(err) => log::warn!( + "Module {} exit failure: {:?}", + self.task_launcher.name(), + err + ), + } self.status = ModuleStatus::Stopped; Ok(()) } else { From 5e4ca1573a27a671a5a82b1c7c41c7a282961005 Mon Sep 17 00:00:00 2001 From: Matteo Nardi Date: Thu, 2 Feb 2023 08:47:55 +0100 Subject: [PATCH 3/3] misc: rename status_map to init_map --- crates/bpf-common/include/output.bpf.h | 6 +++--- crates/bpf-common/src/program.rs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bpf-common/include/output.bpf.h b/crates/bpf-common/include/output.bpf.h index ecdaa5c1..f5a36f6d 100644 --- a/crates/bpf-common/include/output.bpf.h +++ b/crates/bpf-common/include/output.bpf.h @@ -47,7 +47,7 @@ struct { __uint(max_entries, 1); } temp_map SEC(".maps"); -// The status map contains configuration status for the eBPF program: +// The init map contains configuration status for the eBPF program: // - The STATUS_INITIALIZED entry indicates the perf event array initialization // status. On startup it's set to 0, when the userspace program opens the perf // event array it's set to 1. The eBPF program emits events only when it's 1. @@ -57,7 +57,7 @@ struct { __type(key, u32); __type(value, u32); __uint(max_entries, 1); -} status_map SEC(".maps"); +} init_map SEC(".maps"); // Get the temporary buffer inside temp_map as a void pointer, this can be cast // to the required event type and filled before submitting it for output. The @@ -76,7 +76,7 @@ static __always_inline void output_event(void *ctx, void *output_map, void *event, int struct_len, int buffer_len) { u32 key = STATUS_INITIALIZED; - u32 *initialization_status = bpf_map_lookup_elem(&status_map, &key); + u32 *initialization_status = bpf_map_lookup_elem(&init_map, &key); if (!initialization_status || !*initialization_status) { // The userspace is not ready yet for events on the perf event array return; diff --git a/crates/bpf-common/src/program.rs b/crates/bpf-common/src/program.rs index f8555033..53cb6a96 100644 --- a/crates/bpf-common/src/program.rs +++ b/crates/bpf-common/src/program.rs @@ -338,10 +338,10 @@ impl Program { .take_map(map_name) .ok_or_else(|| ProgramError::MapNotFound(map_name.to_string()))?; let mut perf_array: AsyncPerfEventArray<_> = AsyncPerfEventArray::try_from(map_resource)?; - let mut status_map = Array::try_from( + let mut init_map = Array::try_from( self.bpf - .take_map("status_map") - .ok_or_else(|| ProgramError::MapNotFound("status_map".to_string()))?, + .take_map("init_map") + .ok_or_else(|| ProgramError::MapNotFound("init_map".to_string()))?, )?; let buffers = online_cpus() @@ -401,7 +401,7 @@ impl Program { } // Signal eBPF program we're ready by setting STATUS_INITIALIZED - if let Err(err) = status_map.set(0, 1_u32, 0) { + if let Err(err) = init_map.set(0, 1_u32, 0) { log::warn!( "Error setting STATUS_INITIALIZED for {}: {:?}", self.name,