From 47c561bba2bc1f2e68304358e7c010c65a80cc62 Mon Sep 17 00:00:00 2001 From: swananan Date: Thu, 30 Jul 2026 18:10:50 +0800 Subject: [PATCH] fix: handle trace event budget exhaustion Reject undersized and statically over-budget trace events before attach. Treat nested payloads atomically so truncated events remain structurally valid, and cover the failure modes with end-to-end tests. --- config-zh.toml | 1 + config.toml | 1 + docs/configuration.md | 4 +- docs/zh/configuration.md | 4 +- e2e-tests/tests/rust_script_execution.rs | 143 ++++++++++++++++++ .../src/ebpf/codegen/format/mod.rs | 47 ++++-- ghostscope-compiler/src/ebpf/context.rs | 6 + ghostscope-compiler/src/ebpf/instruction.rs | 48 +++++- ghostscope-compiler/src/lib.rs | 9 ++ ghostscope/src/config/settings.rs | 34 +++++ 10 files changed, 279 insertions(+), 18 deletions(-) diff --git a/config-zh.toml b/config-zh.toml index e33d62ce..3462f7c0 100644 --- a/config-zh.toml +++ b/config-zh.toml @@ -255,6 +255,7 @@ mem_dump_cap = 256 # 单条 trace 事件消息的最大大小（字节）。 # 适用于 PerfEventArray 模式的累计缓冲区，也作为内核端构造打印指令时的上界。 +# 最小值为 36 字节（事件协议封装）；脚本指令还需要额外空间。 # 如需在单个事件中打印较多/较大的变量，可适当增大。 max_trace_event_size = 32768 diff --git a/config.toml b/config.toml index 2d437a12..e6aa5042 100644 --- a/config.toml +++ b/config.toml @@ -277,6 +277,7 @@ mem_dump_cap = 256 # Maximum size (in bytes) of a single trace event message. # Applies to PerfEventArray accumulation buffer and serves as an upper bound # when constructing print instructions in the kernel. +# Minimum: 36 bytes for the event envelope; scripts require additional space. # Increase if you plan to print many/large variables in one event. max_trace_event_size = 32768 diff --git a/docs/configuration.md b/docs/configuration.md index e136e159..0cc41a66 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -514,7 +514,8 @@ mem_dump_cap = 256 # Default: 64 bytes. compare_cap = 64 -# Maximum size of a single trace event (bytes). Applies to PerfEventArray accumulation buffer. +# Maximum size of a single trace event (bytes). The protocol envelope requires +# at least 36 bytes; each script's instructions require additional space. max_trace_event_size = 32768 # Recommended values: @@ -778,6 +779,7 @@ GhostScope validates configuration at startup: - **proc_module_offsets_max_entries**: Must be in range 64-65536 - **backtrace_depth**: Must be in range 1-128 frames - **backtrace_unwind_rows_max_entries**: Must be in range 1024-1048576 rows + - **max_trace_event_size**: Must be at least 36 bytes; individual scripts may require more. - **mem_dump_cap**, **compare_cap**, and **max_trace_event_size** are runtime caps; `max_trace_event_size` may be clamped by the selected event transport. Invalid configuration will produce clear error messages with suggestions for fixes. diff --git a/docs/zh/configuration.md b/docs/zh/configuration.md index 7349b9e5..f441a7f4 100644 --- a/docs/zh/configuration.md +++ b/docs/zh/configuration.md @@ -503,7 +503,8 @@ mem_dump_cap = 256 # 默认：64 字节。 compare_cap = 64 -# 单条 trace 事件的最大大小（字节）。适用于 PerfEventArray 累计缓冲区。 +# 单条 trace 事件的最大大小（字节）。协议封装至少需要 36 字节， +# 每个脚本的指令还需要额外空间。 max_trace_event_size = 32768 # 推荐值： @@ -757,6 +758,7 @@ GhostScope 在启动时验证配置： - **proc_module_offsets_max_entries**：必须在 64-65536 范围内 - **backtrace_depth**：必须在 1-128 栈帧范围内 - **backtrace_unwind_rows_max_entries**：必须在 1024-1048576 rows 范围内 + - **max_trace_event_size**：必须至少为 36 字节；具体脚本可能需要更多空间。 - **mem_dump_cap**、**compare_cap** 和 **max_trace_event_size** 是运行时上限；`max_trace_event_size` 可能会根据实际事件传输方式被 clamp。 无效配置将产生清晰的错误消息和修复建议。 diff --git a/e2e-tests/tests/rust_script_execution.rs b/e2e-tests/tests/rust_script_execution.rs index c7b98b02..78e53992 100644 --- a/e2e-tests/tests/rust_script_execution.rs +++ b/e2e-tests/tests/rust_script_execution.rs @@ -2156,6 +2156,149 @@ max_trace_event_size = 32768 Ok(()) } +#[tokio::test] +async fn test_rust_script_nested_event_truncation_stays_structurally_valid() -> anyhow::Result<()> { + init(); + + let target = spawn_rust_global_program().await?; + let script = r#" +trace observe_nested_owners { + print "RNESTED_EVENT_BUDGET:{}:{}", rc, arc; +} +"#; + let result = common::runner::GhostscopeRunner::new() + .with_script(script) + .with_config_content( + r#" +[value_adapters] +max_nesting_depth = 1 +max_sequence_elements = 2 + +[ebpf] +mem_dump_cap = 1024 +max_trace_event_size = 256 +"#, + ) + .attach_to(&target) + .timeout_secs(9) + .enable_sysmon_for_target(false) + .run() + .await; + target.terminate().await?; + let (exit_code, stdout, stderr) = result?; + + assert_eq!(exit_code, 0, "stderr={stderr} stdout={stdout}"); + assert!( + stdout + .lines() + .any(|line| line.contains("RNESTED_EVENT_BUDGET::")), + "Expected structurally valid nested truncation output: {stdout}" + ); + assert!( + !stdout.contains(" anyhow::Result<()> { + init(); + + let target = spawn_rust_global_program().await?; + let script = r#" +trace do_stuff { + print "WIDE:{}:{}:{}:{}:{}:{}", + G_VEC_STRING, G_VEC_VEC_STRING, G_AGGREGATE_OUTER, + G_OPTION_VEC_STRING, G_RESULT_RECORDS_OK, G_CELL_STRING; + print "BYTES:{}:{:s}:{:x}", + G_OWNED_MESSAGE, G_OWNED_MESSAGE, G_OWNED_MESSAGE; +} +"#; + let result = common::runner::GhostscopeRunner::new() + .with_script(script) + .with_config_content( + r#" +[value_adapters] +max_nesting_depth = 1 +max_sequence_elements = 1 + +[ebpf] +mem_dump_cap = 0 +max_trace_event_size = 256 +"#, + ) + .attach_to(&target) + .timeout_secs(5) + .enable_sysmon_for_target(false) + .run() + .await; + target.terminate().await?; + let (exit_code, stdout, stderr) = result?; + let output = format!("{stdout}\n{stderr}"); + + assert_ne!( + exit_code, 0, + "An over-budget trace must fail before attach: {output}" + ); + assert!( + output.contains("Trace event requires at least") + && output.contains("[ebpf].max_trace_event_size is 256 bytes"), + "Expected an actionable event-budget diagnostic: {output}" + ); + assert!( + !stdout.contains("WIDE:") && !stdout.contains("BYTES:"), + "An over-budget trace unexpectedly emitted output: {stdout}" + ); + + Ok(()) +} + +#[tokio::test] +async fn test_trace_event_size_below_protocol_minimum_is_rejected() -> anyhow::Result<()> { + init(); + + let target = spawn_rust_global_program().await?; + let script = r#" +trace do_stuff { + print "TOO_SMALL"; +} +"#; + let result = common::runner::GhostscopeRunner::new() + .with_script(script) + .with_config_content( + r#" +[ebpf] +max_trace_event_size = 0 +"#, + ) + .attach_to(&target) + .timeout_secs(5) + .enable_sysmon_for_target(false) + .run() + .await; + target.terminate().await?; + let (exit_code, stdout, stderr) = result?; + let output = format!("{stdout}\n{stderr}"); + + assert_ne!(exit_code, 0, "An invalid event size must fail: {output}"); + assert!( + output.contains("max_trace_event_size 0 is too small") + && output.contains(&format!( + "Minimum value: {} bytes", + ghostscope_compiler::MIN_TRACE_EVENT_SIZE + )), + "Expected a configuration-level event-size diagnostic: {output}" + ); + assert!( + !output.contains("Invalid argument (os error 22)"), + "The invalid size reached low-level BPF map creation: {output}" + ); + + Ok(()) +} + #[tokio::test] async fn test_rust_script_print_vec_deque_values() -> anyhow::Result<()> { init(); diff --git a/ghostscope-compiler/src/ebpf/codegen/format/mod.rs b/ghostscope-compiler/src/ebpf/codegen/format/mod.rs index d37772aa..f7222434 100644 --- a/ghostscope-compiler/src/ebpf/codegen/format/mod.rs +++ b/ghostscope-compiler/src/ebpf/codegen/format/mod.rs @@ -178,6 +178,10 @@ fn plan_complex_format_layout( headers_total += header_len; let static_payload_len = complex_format_static_payload_len(arg); + let atomic_dynamic_payload_len = match &arg.source { + ComplexArgSource::NestedValue { value, .. } => Some(value.total_len), + _ => None, + }; if let Some(payload_len) = static_payload_len { static_payload_total += payload_len; } else if let ComplexArgSource::MemDumpDynamic { max_len, .. } = &arg.source { @@ -206,7 +210,7 @@ fn plan_complex_format_layout( dynamic_max_lens.push(value.total_len); } - arg_payload_plans.push((header_len, static_payload_len)); + arg_payload_plans.push((header_len, static_payload_len, atomic_dynamic_payload_len)); arg_count = arg_count.saturating_add(1); } @@ -222,14 +226,23 @@ fn plan_complex_format_layout( let args = arg_payload_plans .into_iter() - .map(|(header_len, static_payload_len)| { - let reserved_len = - static_payload_len.unwrap_or_else(|| dynamic_reservations_iter.next().unwrap_or(0)); - ComplexFormatArgLayout { - header_len, - reserved_len, - } - }) + .map( + |(header_len, static_payload_len, atomic_dynamic_payload_len)| { + let reserved_len = static_payload_len + .unwrap_or_else(|| dynamic_reservations_iter.next().unwrap_or(0)); + // Nested sidecars use fixed offsets in their registered presentation. + // A partial reservation cannot be decoded safely, and the emitter + // already represents an omitted nested payload as Truncated. + let reserved_len = match atomic_dynamic_payload_len { + Some(required_len) if reserved_len < required_len => 0, + _ => reserved_len, + }; + ComplexFormatArgLayout { + header_len, + reserved_len, + } + }, + ) .collect::>(); let total_args_payload = args @@ -3045,7 +3058,7 @@ mod complex_format_layout_tests { } #[test] - fn complex_format_layout_bounds_nested_values_to_event_and_wire_budgets() { + fn complex_format_layout_omits_nested_values_that_do_not_fit_atomically() { let context = Context::create(); let args = vec![nested_value_arg(&context, 40_000)]; let max_trace_event_size = 32 * 1024; @@ -3053,11 +3066,21 @@ mod complex_format_layout_tests { let layout = plan_complex_format_layout(max_trace_event_size, 0, &args); let instruction_budget = print_complex_format_instruction_budget(max_trace_event_size, 0); - assert_eq!(layout.total_size, instruction_budget); - assert!(layout.args[0].reserved_len < 40_000); + assert!(layout.total_size < instruction_budget); + assert_eq!(layout.args[0].reserved_len, 0); assert!(layout.inst_data_size <= u16::MAX as usize); } + #[test] + fn complex_format_layout_reserves_nested_values_that_fit_atomically() { + let context = Context::create(); + let args = vec![nested_value_arg(&context, 64)]; + + let layout = plan_complex_format_layout(4096, 0, &args); + + assert_eq!(layout.args[0].reserved_len, 64); + } + #[test] fn short_nested_child_read_errors_stay_within_the_child_slot() { let llvm_ir = short_nested_child_emitter_ir(); diff --git a/ghostscope-compiler/src/ebpf/context.rs b/ghostscope-compiler/src/ebpf/context.rs index 8d21f15a..6a57fe36 100644 --- a/ghostscope-compiler/src/ebpf/context.rs +++ b/ghostscope-compiler/src/ebpf/context.rs @@ -80,6 +80,12 @@ pub enum CodeGenError { }, #[error("Trace context error: {0}")] TraceContext(#[from] ghostscope_protocol::TraceContextOverflow), + #[error( + "Trace event requires at least {required} bytes, but \ + [ebpf].max_trace_event_size is {maximum} bytes. Increase \ + max_trace_event_size or reduce the number or size of trace instructions." + )] + TraceEventBudgetExceeded { required: usize, maximum: usize }, } pub type Result = std::result::Result; diff --git a/ghostscope-compiler/src/ebpf/instruction.rs b/ghostscope-compiler/src/ebpf/instruction.rs index d76e5be9..a0e15e26 100644 --- a/ghostscope-compiler/src/ebpf/instruction.rs +++ b/ghostscope-compiler/src/ebpf/instruction.rs @@ -16,6 +16,23 @@ const fn split_pid_tgid(pid_tgid: u64) -> (u32, u32) { ((pid_tgid >> 32) as u32, pid_tgid as u32) } +fn checked_event_reservation(current: usize, size: u64, maximum: usize) -> Result { + let size = usize::try_from(size).map_err(|_| CodeGenError::TraceEventBudgetExceeded { + required: usize::MAX, + maximum, + })?; + let required = current + .checked_add(size) + .ok_or(CodeGenError::TraceEventBudgetExceeded { + required: usize::MAX, + maximum, + })?; + if required > maximum { + return Err(CodeGenError::TraceEventBudgetExceeded { required, maximum }); + } + Ok(required) +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum RuntimeEarlyReturnReason { AccumulationBufferNull, @@ -63,6 +80,11 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { &mut self, size: u64, ) -> Result>> { + let reserved_upper_bound = checked_event_reservation( + self.compile_time_event_bytes_upper_bound, + size, + self.compile_options.max_trace_event_size as usize, + )?; let i32_ty = self.context.i32_type(); let i64_ty = self.context.i64_type(); @@ -186,9 +208,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { self.builder .build_store(offset_ptr, new_off) .map_err(|e| CodeGenError::LLVMError(format!("Failed to store updated offset: {e}")))?; - self.compile_time_event_bytes_upper_bound = self - .compile_time_event_bytes_upper_bound - .saturating_add(size as usize); + self.compile_time_event_bytes_upper_bound = reserved_upper_bound; Ok(RuntimeReturnAwareValue { value: dest_i8, @@ -712,7 +732,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { #[cfg(test)] mod tests { - use super::split_pid_tgid; + use super::{checked_event_reservation, split_pid_tgid}; + use crate::ebpf::context::CodeGenError; #[test] fn split_pid_tgid_uses_tgid_for_pid_and_pid_for_tid() { @@ -723,4 +744,23 @@ mod tests { assert_eq!(pid, 0x1122_3344); assert_eq!(tid, 0x5566_7788); } + + #[test] + fn event_reservation_accepts_the_exact_budget() { + assert_eq!(checked_event_reservation(28, 14, 42).unwrap(), 42); + } + + #[test] + fn event_reservation_rejects_compile_time_overflow() { + let error = checked_event_reservation(248, 8, 255).unwrap_err(); + + assert!(matches!( + error, + CodeGenError::TraceEventBudgetExceeded { + required: 256, + maximum: 255 + } + )); + assert!(error.to_string().contains("max_trace_event_size")); + } } diff --git a/ghostscope-compiler/src/lib.rs b/ghostscope-compiler/src/lib.rs index abc03de5..e173533d 100644 --- a/ghostscope-compiler/src/lib.rs +++ b/ghostscope-compiler/src/lib.rs @@ -170,6 +170,15 @@ pub const DEFAULT_VALUE_ADAPTER_SEQUENCE_ELEMENTS: usize = 4; /// Maximum configurable number of semantic children per nested sequence. pub const MAX_VALUE_ADAPTER_SEQUENCE_ELEMENTS: usize = 16; +/// Smallest event that can carry the fixed trace envelope and end marker. +/// +/// Individual scripts require additional bytes for their trace instructions. +pub const MIN_TRACE_EVENT_SIZE: u32 = + (std::mem::size_of::() + + std::mem::size_of::() + + std::mem::size_of::() + + std::mem::size_of::()) as u32; + /// Compilation options including save options and eBPF map configuration #[derive(Debug, Clone)] pub struct CompileOptions { diff --git a/ghostscope/src/config/settings.rs b/ghostscope/src/config/settings.rs index 4626e796..623561f6 100644 --- a/ghostscope/src/config/settings.rs +++ b/ghostscope/src/config/settings.rs @@ -603,6 +603,18 @@ impl EbpfConfig { )); } + if self.max_trace_event_size < ghostscope_compiler::MIN_TRACE_EVENT_SIZE { + return Err(anyhow::anyhow!( + "❌ Invalid eBPF configuration in '{}':\n\n\ + max_trace_event_size {} is too small\n\n\ + 💡 Minimum value: {} bytes for the trace event envelope; \ + individual scripts may require more", + file_path, + self.max_trace_event_size, + ghostscope_compiler::MIN_TRACE_EVENT_SIZE + )); + } + // Validate proc_module_offsets_max_entries is reasonable (at least 64, at most 65536) if self.proc_module_offsets_max_entries < 64 || self.proc_module_offsets_max_entries > 65536 { @@ -1055,6 +1067,28 @@ mod tests { assert!(error.to_string().contains("backtrace_depth")); } + #[test] + fn ebpf_rejects_trace_event_size_below_protocol_minimum() { + let mut config = Config::default().ebpf; + config.max_trace_event_size = ghostscope_compiler::MIN_TRACE_EVENT_SIZE - 1; + + let error = config + .validate("test.toml") + .expect_err("undersized trace events should be rejected"); + + let message = error.to_string(); + assert!(message.contains("max_trace_event_size")); + assert!(message.contains(&ghostscope_compiler::MIN_TRACE_EVENT_SIZE.to_string())); + } + + #[test] + fn ebpf_accepts_protocol_minimum_trace_event_size() { + let mut config = Config::default().ebpf; + config.max_trace_event_size = ghostscope_compiler::MIN_TRACE_EVENT_SIZE; + + config.validate("test.toml").unwrap(); + } + #[test] fn ebpf_rejects_legacy_shared_lib_sysmon_key() { let error = toml::from_str::(