From 2b3ee47e795f54fe0e1fe6822ab25919eabb5147 Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Wed, 22 Jul 2026 17:00:14 -0400 Subject: [PATCH] fix(evt): some correctness changes around cwd/perms/timestamps --- rust-toolchain.toml | 3 + src/base_plugin/mod.rs | 48 ++++------- src/parsers.rs | 181 +++++++++++++++++++++++++++++++---------- src/threadstate.rs | 11 ++- 4 files changed, 166 insertions(+), 77 deletions(-) create mode 100644 rust-toolchain.toml diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000..bc69bc5 --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "1.95.0" +components = ["rustfmt", "rust-std", "clippy"] diff --git a/src/base_plugin/mod.rs b/src/base_plugin/mod.rs index 325e6c3..315bfaf 100644 --- a/src/base_plugin/mod.rs +++ b/src/base_plugin/mod.rs @@ -319,39 +319,19 @@ impl EderaPlugin { } pub fn extract_is_open_read(&mut self, mut req: ExtractRequest) -> Result { - self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - if let Ok(res) = parsers::get_openstate(zone_evt) { - return Ok(res == parsers::OpenType::Read); - } - Ok(false) - }) + self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| Ok(parsers::is_open_read(zone_evt))) } pub fn extract_is_open_write(&mut self, mut req: ExtractRequest) -> Result { - self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - if let Ok(res) = parsers::get_openstate(zone_evt) { - return Ok(res == parsers::OpenType::Write); - } - Ok(false) - }) + self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| Ok(parsers::is_open_write(zone_evt))) } pub fn extract_is_open_exec(&mut self, mut req: ExtractRequest) -> Result { - self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - if let Ok(res) = parsers::get_openstate(zone_evt) { - return Ok(res == parsers::OpenType::Exec); - } - Ok(false) - }) + self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| Ok(parsers::is_open_exec(zone_evt))) } pub fn extract_is_open_create(&mut self, mut req: ExtractRequest) -> Result { - self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - if let Ok(res) = parsers::get_openstate(zone_evt) { - return Ok(res == parsers::OpenType::Create); - } - Ok(false) - }) + self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| Ok(parsers::is_open_create(zone_evt))) } pub fn extract_count(&mut self, _: ExtractRequest) -> Result { @@ -935,7 +915,7 @@ impl EderaPlugin { self.threadstate .with_threadinfo(&evt.zone_id, &evt.thread_id, |tinfo| { if tinfo.clone_ts != 0 { - Some(evt.timestamp - tinfo.clone_ts) + Some(evt.timestamp.saturating_sub(tinfo.clone_ts)) } else { None } @@ -1154,7 +1134,11 @@ impl EderaPlugin { if evt.event_type == event_codes::PPME_SCHEDSWITCH_1_E as u32 || evt.event_type == event_codes::PPME_SCHEDSWITCH_6_E as u32 { - Some(exec_time.last_switch_ts - exec_time.previous_switch_ts) + Some( + exec_time + .last_switch_ts + .saturating_sub(exec_time.previous_switch_ts), + ) } else { // TODO(bml) libsinsp only reports this for explicit switch events, // we could actually do better, but for now maintain strict compat @@ -1207,12 +1191,12 @@ impl EderaPlugin { .filter(|s| !s.is_empty()) .find_map(|entry| { // Each entry is "subsystem=path" - if let Some((name, path)) = entry.split_once('=') - && (name == subsystem || name == format!("{}_cgroup", subsystem)) - { - CString::new(path.to_string()).ok(); + let (name, path) = entry.split_once('=')?; + if name == subsystem || name == format!("{}_cgroup", subsystem) { + CString::new(path.to_string()).ok() + } else { + None } - None }) }) .unwrap_or(CString::new("NA").expect("default value must parse"))) @@ -2422,7 +2406,7 @@ impl EderaPlugin { .and_then(|evt| { self.with_nth_parent_proc_thread(&evt.zone_id, &evt.thread_id, 1, |atinfo| { if atinfo.clone_ts != 0 { - Some(evt.timestamp - atinfo.clone_ts) + Some(evt.timestamp.saturating_sub(atinfo.clone_ts)) } else { None } diff --git a/src/parsers.rs b/src/parsers.rs index bd35dcf..00e7ed2 100644 --- a/src/parsers.rs +++ b/src/parsers.rs @@ -21,53 +21,83 @@ const FLAGS_SOCKET_CONNECTED: u32 = 1 << 13; const FLAGS_OVERLAY_UPPER: u32 = 1 << 17; const FLAGS_OVERLAY_LOWER: u32 = 1 << 18; -#[derive(PartialEq)] -pub enum OpenType { - Read, - Write, - Exec, - Create, -} - -pub fn get_openstate(evt: &ZoneKernelSyscallEvent) -> Result { - // strum from discriminant to make this simpler - let etype = - event_codes::from_repr(evt.event_type).ok_or(anyhow!("could not parse event type"))?; - - if is_open_file(etype) { - let is_new_version = etype == event_codes::PPME_SYSCALL_OPENAT_2_X - || etype == event_codes::PPME_SYSCALL_OPENAT2_X; - // new versions have open flags at arg 3 insted of arg 2 - let flags = if is_new_version { - u32::from_ne_bytes(evt.event_params[3].param_data.as_slice().try_into()?) - } else { - u32::from_ne_bytes(evt.event_params[2].param_data.as_slice().try_into()?) - }; +// Open access/create/exec classification mirrors libsinsp +// `evt.is_open_read/write/exec/create`. +const OPEN_EXEC_MODE_MASK: u32 = + ppm_consts::PPM_S_IXUSR | ppm_consts::PPM_S_IXGRP | ppm_consts::PPM_S_IXOTH; + +fn read_u32_param(evt: &ZoneKernelSyscallEvent, idx: usize) -> Option { + let param = evt.event_params.get(idx)?; + Some(u32::from_ne_bytes( + param.param_data.as_slice().try_into().ok()?, + )) +} - if (flags & ppm_consts::PPM_O_RDONLY) != 0 { - return Ok(OpenType::Read); - } else if (flags & ppm_consts::PPM_O_WRONLY) != 0 { - return Ok(OpenType::Write); - } else if (flags & ppm_consts::PPM_O_F_CREATED) != 0 { - return Ok(OpenType::Create); - } else if (flags & (ppm_consts::PPM_O_TMPFILE | ppm_consts::PPM_O_CREAT)) != 0 - && etype != event_codes::PPME_SYSCALL_OPEN_BY_HANDLE_AT_X - { - let mode_bits = if is_new_version { - u32::from_ne_bytes(evt.event_params[4].param_data.as_slice().try_into()?) - } else { - u32::from_ne_bytes(evt.event_params[3].param_data.as_slice().try_into()?) +/// Normalized open flags for an open-family exit event (the `flags` param), or +/// `None` if this isn't such an event or the param is absent/malformed. The +/// modern openat variants carry flags at arg 3, the legacy forms at arg 2. +fn open_flags(evt: &ZoneKernelSyscallEvent, etype: event_codes) -> Option { + let idx = match etype { + event_codes::PPME_SYSCALL_OPENAT_2_X | event_codes::PPME_SYSCALL_OPENAT2_X => 3, + event_codes::PPME_SYSCALL_OPEN_X | event_codes::PPME_SYSCALL_OPEN_BY_HANDLE_AT_X => 2, + _ => return None, + }; + read_u32_param(evt, idx) +} + +pub fn is_open_read(evt: &ZoneKernelSyscallEvent) -> bool { + let Some(etype) = event_codes::from_repr(evt.event_type) else { + return false; + }; + open_flags(evt, etype).is_some_and(|flags| (flags & ppm_consts::PPM_O_RDONLY) != 0) +} + +pub fn is_open_write(evt: &ZoneKernelSyscallEvent) -> bool { + let Some(etype) = event_codes::from_repr(evt.event_type) else { + return false; + }; + open_flags(evt, etype).is_some_and(|flags| (flags & ppm_consts::PPM_O_WRONLY) != 0) +} + +pub fn is_open_create(evt: &ZoneKernelSyscallEvent) -> bool { + let Some(etype) = event_codes::from_repr(evt.event_type) else { + return false; + }; + let Some(flags) = open_flags(evt, etype) else { + return false; + }; + // O_F_CREATED means the file was created; O_TMPFILE creates one only on success. + (flags & ppm_consts::PPM_O_F_CREATED) != 0 + || ((flags & ppm_consts::PPM_O_TMPFILE) != 0 && get_retval(evt).is_some_and(|r| r >= 0)) +} + +pub fn is_open_exec(evt: &ZoneKernelSyscallEvent) -> bool { + let Some(etype) = event_codes::from_repr(evt.event_type) else { + return false; + }; + // `creat` carries mode at arg 2; open-family carries it at arg 3 (legacy) or + // arg 4 (modern), and only counts as exec when the open can create the file. + // open_by_handle_at has no mode param and is excluded, as in libsinsp. + let mode_idx = match etype { + event_codes::PPME_SYSCALL_CREAT_X => 2, + event_codes::PPME_SYSCALL_OPEN_X + | event_codes::PPME_SYSCALL_OPENAT_2_X + | event_codes::PPME_SYSCALL_OPENAT2_X => { + let Some(flags) = open_flags(evt, etype) else { + return false; }; - if (mode_bits - & (ppm_consts::PPM_S_IXUSR | ppm_consts::PPM_S_IXGRP | ppm_consts::PPM_S_IXOTH)) - != 0 - { - return Ok(OpenType::Exec); + if (flags & (ppm_consts::PPM_O_TMPFILE | ppm_consts::PPM_O_CREAT)) == 0 { + return false; + } + if etype == event_codes::PPME_SYSCALL_OPEN_X { + 3 + } else { + 4 } } - } - - Err(anyhow!("not an open event")) + _ => return false, + }; + read_u32_param(evt, mode_idx).is_some_and(|mode| (mode & OPEN_EXEC_MODE_MASK) != 0) } pub fn is_enter(evt: &ZoneKernelSyscallEvent) -> bool { @@ -436,4 +466,69 @@ mod tests { assert_eq!(get_retval(&evt), None); assert_eq!(syscall_failed(&evt), None); } + + // constructs a modern openat exit event: params are [fd, dirfd, name, flags, mode]. + // fd=3 (success) so the O_TMPFILE create path is satisfied when exercised. + fn openat2x_event(flags: u32, mode: u32) -> ZoneKernelSyscallEvent { + let u32_param = |v: u32| ZoneKernelEventParam { + param_data: v.to_ne_bytes().to_vec(), + ..Default::default() + }; + ZoneKernelSyscallEvent { + event_type: event_codes::PPME_SYSCALL_OPENAT_2_X as u32, + event_category: "EC_FILE | EC_SYSCALL".to_string(), + event_params: vec![ + u32_param(3), + u32_param(0), + u32_param(0), + u32_param(flags), + u32_param(mode), + ], + ..Default::default() + } + } + + #[test] + fn rdonly_open_is_read_only() { + let evt = openat2x_event(ppm_consts::PPM_O_RDONLY, 0); + assert!(is_open_read(&evt)); + assert!(!is_open_write(&evt)); + } + + #[test] + fn wronly_open_is_write_only() { + let evt = openat2x_event(ppm_consts::PPM_O_WRONLY, 0); + assert!(!is_open_read(&evt)); + assert!(is_open_write(&evt)); + } + + #[test] + fn rdwr_open_is_both_read_and_write() { + let evt = openat2x_event(ppm_consts::PPM_O_RDWR, 0); + assert!(is_open_read(&evt)); + assert!(is_open_write(&evt)); + } + + #[test] + fn created_flag_is_create_independent_of_access_mode() { + let evt = openat2x_event(ppm_consts::PPM_O_WRONLY | ppm_consts::PPM_O_F_CREATED, 0); + assert!(is_open_create(&evt)); + assert!(is_open_write(&evt)); + } + + #[test] + fn creating_open_with_exec_mode_is_exec() { + let evt = openat2x_event( + ppm_consts::PPM_O_WRONLY | ppm_consts::PPM_O_CREAT, + ppm_consts::PPM_S_IXUSR, + ); + assert!(is_open_exec(&evt)); + } + + #[test] + fn plain_read_open_is_neither_create_nor_exec() { + let evt = openat2x_event(ppm_consts::PPM_O_RDONLY, 0); + assert!(!is_open_create(&evt)); + assert!(!is_open_exec(&evt)); + } } diff --git a/src/threadstate.rs b/src/threadstate.rs index 9eeaf6c..1c685cc 100644 --- a/src/threadstate.rs +++ b/src/threadstate.rs @@ -2089,6 +2089,12 @@ impl ZoneInfo { } fn parse_chdir_exit(&mut self, event: &ZoneKernelSyscallEvent) -> Result<()> { + // if syscall failed, just bail, we didn't actually change the cwd + if parsers::get_retval(event).filter(|&v| v >= 0).is_none() { + debug!("no success retval found for chdir event: {:?}", event); + return Ok(()); + }; + // if we have a thread for this event, update the cwd of that thread, otherwise NBD self.with_mut_threadinfo_ctx(event, |tinfo| { tinfo.cwd = event.event_params[1].param_pretty.clone(); // pretty version is already a string @@ -2449,8 +2455,9 @@ impl ZoneInfo { ExecTime { last_switch_ts: event.timestamp, previous_switch_ts: exectime.last_switch_ts, - cumulative_switch_time: exectime.cumulative_switch_time - + (event.timestamp - exectime.last_switch_ts), + cumulative_switch_time: exectime + .cumulative_switch_time + .saturating_add(event.timestamp.saturating_sub(exectime.last_switch_ts)), }, );