From e85039efc056e2c97d53507bcbb0c4a4ecc4dce2 Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Sat, 8 Aug 2026 14:19:42 -0700 Subject: [PATCH] fix(agent): make Endian::u32/i32 total in the BTF parser (ADR-0014) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endian::u32 did b.try_into().expect("4-byte slice") on blob-derived slices — the one expect-panic on the untrusted-blob parse path. Unreachable today (all callers pass length-guarded 4-byte subslices), but ADR-0014's contract is never panic on the blob, degrade gracefully. A future wrong-sized caller would turn a malformed or truncated /sys/kernel/btf/vmlinux into an agent crash-loop instead of a fail-closed preflight. Make Endian::u32 (and i32, which delegates to it) return Result<_, BtfParseError>, using the existing BtfParseError::Truncated variant, and thread the Result through all 13 call sites with `?` — they already sit inside functions returning the parser's error type and fold to fail_closed(). Well-formed BTF blobs parse identically; only a malformed/short blob now yields a fail-closed preflight instead of a panic. Adds a direct totality test for Endian::u32/i32 on too-short slices, plus a blob-level regression test (built with the existing BtfBuilder fixture, then truncated mid-integer) confirming RawBtf::parse fails closed rather than panicking. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP --- agent/protector-agent/src/preflight/btf.rs | 52 +++++++++++-------- .../src/preflight/btf_tests.rs | 35 +++++++++++++ 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/agent/protector-agent/src/preflight/btf.rs b/agent/protector-agent/src/preflight/btf.rs index 7de1a1b..22e7b69 100644 --- a/agent/protector-agent/src/preflight/btf.rs +++ b/agent/protector-agent/src/preflight/btf.rs @@ -72,16 +72,24 @@ enum Endian { } impl Endian { - fn u32(self, b: &[u8]) -> u32 { - let a: [u8; 4] = b.try_into().expect("4-byte slice"); - match self { + /// Total: a slice shorter than 4 bytes yields `Truncated` rather than panicking — the + /// one integer reader on the blob-derived parse path (ADR-0014's never-panic, + /// degrade-gracefully contract). `get(..4)` bounds-checks the length, so the + /// subsequent `try_into` is infallible. + fn u32(self, b: &[u8]) -> Result { + let a: [u8; 4] = b + .get(..4) + .ok_or(BtfParseError::Truncated)? + .try_into() + .unwrap(); + Ok(match self { Self::Little => u32::from_le_bytes(a), Self::Big => u32::from_be_bytes(a), - } + }) } - fn i32(self, b: &[u8]) -> i32 { - self.u32(b) as i32 + fn i32(self, b: &[u8]) -> Result { + self.u32(b).map(|v| v as i32) } } @@ -142,11 +150,11 @@ impl RawBtf { } else { return Err(BtfParseError::BadMagic); }; - let hdr_len = endian.u32(&data[4..8]) as usize; - let type_off = endian.u32(&data[8..12]) as usize; - let type_len = endian.u32(&data[12..16]) as usize; - let str_off = endian.u32(&data[16..20]) as usize; - let str_len = endian.u32(&data[20..24]) as usize; + let hdr_len = endian.u32(&data[4..8])? as usize; + let type_off = endian.u32(&data[8..12])? as usize; + let type_len = endian.u32(&data[12..16])? as usize; + let str_off = endian.u32(&data[16..20])? as usize; + let str_len = endian.u32(&data[20..24])? as usize; let type_start = hdr_len .checked_add(type_off) @@ -191,9 +199,9 @@ impl RawBtf { if buf.len() < 12 { return Err(BtfParseError::Truncated); } - let name_off = endian.u32(&buf[0..4]); - let info = endian.u32(&buf[4..8]); - let extra = endian.u32(&buf[8..12]); + let name_off = endian.u32(&buf[0..4])?; + let info = endian.u32(&buf[4..8])?; + let extra = endian.u32(&buf[8..12])?; let kind = ((info >> 24) & 0x1f) as u8; let kind_flag = (info >> 31) & 1 == 1; let vlen = (info & 0xffff) as usize; @@ -208,9 +216,9 @@ impl RawBtf { let mut members = Vec::with_capacity(vlen); for i in 0..vlen { let base = consumed + i * 12; - let m_name = endian.u32(&buf[base..base + 4]); - let m_type = endian.u32(&buf[base + 4..base + 8]); - let m_off = endian.u32(&buf[base + 8..base + 12]); + let m_name = endian.u32(&buf[base..base + 4])?; + let m_type = endian.u32(&buf[base + 4..base + 8])?; + let m_off = endian.u32(&buf[base + 8..base + 12])?; // kind_flag set ⇒ the low 24 bits are the bit-offset (high 8 the // bitfield size, discarded); unset ⇒ the whole word is the plain // bit-offset. Either way this crate's targets are never @@ -241,8 +249,8 @@ impl RawBtf { let mut variants = Vec::with_capacity(vlen); for i in 0..vlen { let base = consumed + i * 8; - let v_name = endian.u32(&buf[base..base + 4]); - let v_val = endian.i32(&buf[base + 4..base + 8]); + let v_name = endian.u32(&buf[base..base + 4])?; + let v_val = endian.i32(&buf[base + 4..base + 8])?; variants.push(EnumVariant { name: Self::string_at(strings, v_name), value: v_val as i64, @@ -259,9 +267,9 @@ impl RawBtf { let mut variants = Vec::with_capacity(vlen); for i in 0..vlen { let base = consumed + i * 12; - let v_name = endian.u32(&buf[base..base + 4]); - let lo = endian.u32(&buf[base + 4..base + 8]) as u64; - let hi = endian.u32(&buf[base + 8..base + 12]) as u64; + let v_name = endian.u32(&buf[base..base + 4])?; + let lo = endian.u32(&buf[base + 4..base + 8])? as u64; + let hi = endian.u32(&buf[base + 8..base + 12])? as u64; variants.push(EnumVariant { name: Self::string_at(strings, v_name), value: ((hi << 32) | lo) as i64, diff --git a/agent/protector-agent/src/preflight/btf_tests.rs b/agent/protector-agent/src/preflight/btf_tests.rs index 04d7298..7070c39 100644 --- a/agent/protector-agent/src/preflight/btf_tests.rs +++ b/agent/protector-agent/src/preflight/btf_tests.rs @@ -30,6 +30,41 @@ fn rejects_a_type_section_that_overruns_the_blob() { )); } +#[test] +fn endian_u32_and_i32_are_total_on_a_too_short_slice() { + // Regression for the ADR-0014 never-panic contract: `Endian::u32`/`i32` used to + // `.expect()` a 4-byte slice. Every current caller happens to pass one, but a + // wrong-sized caller (or a future one) must fail closed with `Truncated`, not panic. + assert!(matches!( + Endian::Little.u32(&[1, 2, 3]), + Err(BtfParseError::Truncated) + )); + assert!(matches!( + Endian::Big.u32(&[]), + Err(BtfParseError::Truncated) + )); + assert!(matches!( + Endian::Little.i32(&[0u8; 2]), + Err(BtfParseError::Truncated) + )); +} + +#[test] +fn a_blob_truncated_mid_integer_fails_closed_without_panicking() { + // A well-formed blob (built by the same fixture builder the other tests use), then + // cut short a couple of bytes into its type section — the truncation lands + // mid-integer, inside the last member's byte_offset word, not on a type boundary. + // The parser must fail closed (Truncated), never panic, on this malformed input. + let mut b = BtfBuilder::new(); + b.add_struct("demo_file", false, &[("f_flags", 0, 40), ("f_path", 0, 64)]); + let mut blob = b.build(); + blob.truncate(blob.len() - 2); + assert!(matches!( + RawBtf::parse(&blob), + Err(BtfParseError::Truncated) + )); +} + #[test] fn finds_a_plain_struct_field_offset() { let mut b = BtfBuilder::new();